On Tue, Nov 4, 2008 at 7:30 PM, Anselm <span dir="ltr"></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hello,<br>
<br>
Having followed this thread for a while, I decided to do my bit, and start<br>
reviewing patches. I didn't get very far ; I stopped at the first patch.<br>
<br>
Not because of the code - I could understand what the original code did, and<br>
what the patched code did - but because there are two decisions to be made :<br>
<br>
1. In that case - and I guess this is not uncomon - the old behaviour might be<br>
a bug, but it might have been by design. I have no way of telling.</blockquote><div><br>A lot of core patches are trying to improve the API (or some other improvement) rather than fix a bug per se. <br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
<br>
2. Changing the old behaviour to the new one could break old modules that<br>
implicitely relied on the old behaviour. Again, this is probably not uncomon.<br>
How do I weight the importance of the patch vs stability of old modules ?</blockquote><div><br>In general we don't maintain backwards compatibility of code between major releases (although neither do we break it just for fun) - every release has detailed upgrade instructions for module maintainers as <a href="http://drupal.org/update/modules">http://drupal.org/update/modules</a><br>
<br>So changing function names, function parameters - this is all fair game as long as there's a tangible improvement to doing so.<br><br><br><br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
So what do I do next ? I don't have a sufficient overview of that part of<br>
Drupal to make such decisions myself. Should I mark it as reviewed, and add<br>
as a note what the potential implications are ?</blockquote><div><br>Making a note of what the potential implications are is sensible where you're not sure where something should go - this will help the next person looking at the issue to make sense of what specific questions need to be answered in relation to the patch.<br>
<br>The other thing you can do is join #drupal on irc and try to find someone more familiar with that part of Drupal - if you say 'I'm reviewing a patch about x but I have some questions' you'll usually get help.<br>
<br>Nat<br></div></div><br>