Drupalchix:TwinCities Patch Party
who: twincities drupalchix
what: drupal patch party
when: (tentatively) August 8, 2009 - Saturday - 10-2 (optional to stay til 4)
where: somewhere with internet
why: group project to learn how to contribute back to drupal project by 'reviewing patches' to drupal 7 codebase
Drupal 7 is in the final weeks of accepting contributions from developers all over the world, many of whom are hoping to get their code into the next release of Drupal. (a.k.a. the "code freeze" is 9/1/09)
Since Drupal code is open source, a very important task is 'Patch Reviewing' by volunteering Drupal community members. The 'Patch Reviewers' offer a critical eye to contributions of code.
Wait, what the heck is a 'patch?' A 'patch' is a file that contains information about edits to the Drupal 7 code. Effectively, a 'patch' is a unit of communication among Drupal developers. These 'patch' files mostly say "delete these lines of code and replace them with these other lines of code." The patches could contain text that explains the code. The patches could be installed in Drupal to see if they break Drupal 7. Reading the 'patch review submission' issue queue will give some backstory to the patch.
Patch reviewers ensure that the code is good, based on a number of criteria. This ensures the overall quality of Drupal 7.
This event is geared for all levels of Drupal-interested women. So don't worry, you can help even if you do not know PHP (though be warned, we will be looking at code the whole day. This might be a good day to look under the hood in a supportive environment!)
About CVS: patches can just get uploaded to Drupal website. Therefore, CVS is helpful, but not required. Some of us will have CVS at the sprint and we can make do. We anticipate that there will be a lot of working in little groups.
We will be working through this 'patch review' process together as a drupalchix:tc first group project! Yay!
how long: 4-6 hours
what to bring: computer, snacks
programs you might want on your computer: ALL OPTIONAL!!
text editor, code editor, localhost, irc program, tortoise cvs (windows), xcode (mac)(easy way to get cvs client) (let us know if you have questions!)
how many: 3-15 people
#drupalchix solidarity committee and allied service auxiliary force: (a.k.a. male allies) The in-person event is for women-identified (&genderqueer) drupalchix. There can be a virtual sprint of remote #drupalchix & allied forces at the same time (TBD).
things we will probably need:
- internet
- power strips
- cvs account - not necessary
- checked out copies of latest (to the very moment) drupal7 (obtained via cvs)
- cvs client to run patches against installs of drupal
- this is the patch queue: http://drupal.org/project/issues/search/drupal?version[0]=156281&status[0]=8&status[1]=14>
references
http://webchick.net/6-pass-patch-reviews
Ical feed
thx
thanks to webchick, arianek, mndonx, bangpound, zzolo & the twin cities drupalchix for encouraging this!
nice work organizing this
nice work organizing this chach!
that be early on the west coast, but i'll try and join in virtually for a bit later on :-)
yeah
mndonx has to go earlier that day -- but later on will be awesome! :)
Oh no! I am going to be out
Oh no! I am going to be out of town traveling on that day. I totally would have come otherwise. Good luck--I'm sure it will be a success!
location & reminder
Hey anyone who is planning on coming tomorrow...
We will meet at 10am. (Sorry for not posting the location sooner!)
Common Roots Cafe
www.commonrootscafe.com
2558 Lyndale Ave S
Minneapolis, MN 55405
We will be on IRC on and off.
This is what I know we'll be doing:
installing drupal 7 locally
going over the patch review process
looking at the patch issue queue & connecting up with drupal people on IRC to find issues we can work on tomorrow.
There will probably be a repeat patch party next weekend as well...we'll keep you posted.
Chach, Looks fun but I'm
Chach,
Looks fun but I'm looking to attend events that start after 12pm. Don't parties start when it gets dark? Hooray for pizza, beer and technology!
update, noon o'clock
we're still doing stuff, but mostly online now.
here's an update
* mndonx & i showed up (we even rode our bikes in the rain to get to the coffeeshop)
* future meetings won't be quite so early - but it was good to wake up.
* we used 'smartcvs' to get the latest version of drupal 7 - a nice cvs client for mac os x, has a gui. this is good if you haven't installed xcode on your mac. (which is like 1GB download, probably not so great for patch parties at coffeeshops!)
* we looked at: http://webchick.net/6-pass-patch-reviews
* we tried to make sense of the patch issue queue
* after about an hour we had started to figure out what parts of the patches we should look at. (we looked at a little patch & a bigger, more confusing one
About the patch issue queue
It's pretty hard to make sense of the patch issue queue. Honestly, we had the same exact problem at the documentation sprint. It would be good to come up with a short list ahead of time of easy patches to review...however, I haven't figured that out yet. We might do this again next weekend, so maybe it will make more sense by then.
That said, these are some of the ways that I tried to find some patches to review (no success yet):
These are the patches that fix bug reports:
http://drupal.org/project/issues/search/drupal?text=&assigned=&submitted=&participant=&status[]=8&status[]=14&categories[]=bug&version[]=156281&issue_tags=
Ignore issues with lots of replies. (such as 180 replies) They are probably good to look at to get a sense of what people are trying to fix in Drupal -- but the fact that the issue is complex probably means it is not good for a first patch to review. I think if there are 180 replies to an issue that it means that other people tried to also review the patch...but there's still a problem.
Ignore the issues that failed the test bots.
I found a page that looks like this: http://testing.drupal.org/pifr/file/1/471744.patch -- someone tried testing their patch but I can't do tests yet, so I'm not even sure why it failed.
Small patches not necessarily easier! One patch we looked at seemed really simple, but it wasn't: http://drupal.org/files/issues/471744.patch (we were also looking at a bigger patch to try to figure out how awful a patch could be)
It is obvious that the patch we looked at just adds a pass-by-reference (an &) to a variable, and there are some comments in the patch. That part made a lot of sense.
However, the discussion thread (http://drupal.org/node/471744) was talking about whether or not it is a good idea to pass by reference or not. That gets into php4 vs php5... and it is obvious that to really answer this question, you would need to get very involved in the code for that particular issue. So I will be looking at some other patches to see if they are a little less complicated.
Silver lining
That said, already good things have come of trying to review patches:
* we participated on IRC (which is usually daunting)
* we are set up with CVS
* we looked at drupal 7 already
* we obviously understand what a .patch file is
* looking at the patch issue queues is interesting, even if there is no clearcut way to actually help
* we should really plan a twin cities meeting about 'testing' because understanding testing would help make the patch review process make more sense.
* picking a time to actually concentrate on things that sound daunting is good...even if you don't make a ton of progress. eventually we'll get it, and working through this will make it easier to explain to other people in the twin cities & in drupalchix.
but today isn't over yet!
if anyone has other tips about how to pick patches to review...do let us know!
If you're looking for
If you're looking for simpler patches to start with searching for issues tagged with Novice, or Quick fix is always a good place to look.
http://drupal.org/project/issues/search/drupal?text=&assigned=&submitted...
One of the things I like to do with bigger patches when I'm not feeling up to analyzing 1000 lines of code is to just pick one aspect of the patch review process to focus on. Something like, I'm just going to go through and make sure everything is grammatically correct. Not even bother with wether it works or not.
Thanks!
That is definitely a good idea about the QuickFix/Novice tags.
This is what I am trying to figure out right now:
I found an issue from the queue that has some obvious problems (functions are missing comments entirely). I think the next step is to mark it as 'needs work' and leave a comment about why. It would probably be a good idea to look at the patch as much as possible, mark down all the places that need comments, try the patch out... which would help along the process as much as possible before marking it as 'needs work.'
Yeah, once you've found
Yeah, once you've found something take some notes about what is wrong. If I feel qualified to do so I usually like to try and recommend a possible solution. If it's a functionality problem, as in you've applied the patch and discovered something that just doesn't work include steps that can be used to reproduce the error/problem you're encountering.
Then post your comments and mark the patch as "needs work".
I would recommend not getting hung up on trying to make sure you catch everything. Otherwise you'll spend hours and hours on one patch. Hours that might take someone else who knows that aspect of the code just a few minutes to notice is wrong and point out. Obviously the more thorough you can be the better but anything you can point out that needs work helps keep the patch moving forward.
Another tip for finding patches to review. Figure out what parts of Drupal you're most interested in. I know I'm really interested in media and file handling so I will specifically seek out patches that deal with those things. If the patch does something that I really support and want to see I will spend more time reviewing it.
hours later...
One patch reviewed, finally.
http://drupal.org/node/476294#comment-1902884
Sent it back to catch because the patch is missing an important comment.
I found this issue in the 'novice/quick fix' list. This patch adds URL aliases on the user edit page for drupal 7. I applied the patch to my recently checked out version of drupal 7. I had to enable & disable a module to get the url alias feature to show up on the user edit page. Someone else had already tested this feature as well, so I replicated the steps they made.
I took notes as I went through the code -- there were a bunch of things in drupal 7 that I wasn't sure if they were right. I also found a bunch of bad documentation practices. I'm not sure why form_alter functions only get the tiny snippet of 'implement form_alter function' -- seems kind of lazy.
I tried to make my own patch of the changes I made...but that was not working easily. So instead I just put some notes in the comment about the patch. One does what one can...
The end.
Yay! That's awesome. I would
Yay! That's awesome. It gets quicker and easier each time. I know I spent at least a whole day the first time I tried to review anything. Took me a while to get comfortable ripping apart someone else's code.
I would be happy to spend some time going through creating a patch with you if you're still stuck next time we run into each other. You might have to remind me that I volunteered though. :)
eojthebrave gave really good advice!
Everything he said was spot on. Don't worry about catching everything in one go. At http://webchick.net/6-pass-patch-reviews I wrote up a bunch of passes I do on every patch that I review. Even just doing one of those though is helpful and pushes the patch along.
There are also the community initiative pages at http://drupal.org/community-initiatives/drupal-core where you can drill down on any specific areas, like usability, or field API, etc. but that can be a bit of a toss-up between a 15-minute patch review and a 3-hour patch review. ;) At least you can find something in your general area of interest though!
Also, if you're looking for more "low-hanging fruit" patches, other than quick fix/novice, another place to look is either the "documentation" or "user interface text" components. Feel free to dig around in the components for JS patches or CSS patches, etc. too.
Though honestly, if you're looking for patches to review, the best thing to do is probably just come on IRC, and mention in #drupal that you're looking for patches to review. Unless you come in at a rare dead time, you will probably be inundated with links from very helpful people who would be happy to help you through anything you had problems with. :)