[development] Reviewing patches and making decisions -> Sociocracy could be a way to go!
earnie at users.sourceforge.net
Thu Nov 6 13:16:24 UTC 2008
Quoting Thomas Zahreddin <tz at it-arts.org>:
>> Not because of the code - I could understand what the original code did,
>> and what the patched code did - but because there are two decisions to be
>> made :
>> 1. In that case - and I guess this is not uncomon - the old behaviour might
>> be a bug, but it might have been by design. I have no way of telling.
If it is by design then the code should be documented as to why. If no
comments then we can assume that the patch is either fixing a bug or
create a new feature. If this is a new feature and the issue is marked
as a bug, change the information to feature request from bug report.
Feature requests always go to the next release so be sure to change the
version and ask the patch be rolled for HEAD as well as give
information about the patch itself.
Bug reports usually go to HEAD, you may find an instance of an API call
that has been removed so HEAD can't contain the patch. Once applied to
HEAD then the patch can roll backward to other versions.
> (caution: Ironical!) Oh, I can't count how often i heard: "the code tells it
> all!" or "read my code first" or "only code counts, not the words
> about it" ..
Don't forget about common sense.
> Yes I can read the code but the question above can not be answered by reading
> the code - much more helpful would be a comment what the _aim_ of some lines
> of code is.
But it can be answered by looking at the source. If no comment is
present then there is no reason to think that anything special was
being done by the previous code.
> And maybe the group working on and with that particular module has a list of
> decisions (not a mailinglist) and why they where made - so one could go back
> to the original decision (should be referenced in the code) and look up what
> the original intention was.
Again, common sense. If decisions aren't documented in an issue
somewhere based on the policies already set forth for Drupal core then
a patch isn't really warranted or the decisions have to be stated again
in the issue. The issue is the communication point for the patch. And
as a matter of policy Drupal core patches are never committed without
> Do you agree that these information could help a lot in similar situaltions?
>> 2. Changing the old behaviour to the new one could break old modules that
>> implicitely relied on the old behaviour. Again, this is probably not
>> uncomon. How do I weight the importance of the patch vs stability of old
>> modules ?
> IMHO persons, interested in a module (like programmers and users of this or a
> compatible module), shall have the possiblity to be heard with their
> requirements. It is up to the maintainer (or moderater) to listen to all and
> their arguments what is urgent and /or important and to come up with
> suggestions to which most (if not all) people can agree. The final
> decision is
> up to the maintainer (keeping in mind the long term goals, all persons
> interested in the module and the available resources) in cooperation with the
> person willing to contribute (e.g. code).
It is also up to the interested persons to provide patches for the
modules. Everyone can be a patch creator.
> If the group of persons working on a module keeps a logbook about their
> decisions, then everybody is able to follow the decisions regarding a modul.
Contrib modules are of a different breed of issue. There is no
enforcement of Drupal policy on the contrib module.
>> So what do I do next ? I don't have a sufficient overview of that part of
>> Drupal to make such decisions myself. Should I mark it as reviewed, and add
>> as a note what the potential implications are ? Or is there another process
>> to follow in such situations ?
Leave it as needs review but leave your comments.
> I agree absoluty: We do not have approriate structures in the drupal
> to work efficently, make decissions traceable, keeping the teamspirit up and
> stay open for new developers to join.
I highly disagree. Yes, people will get their feelings hurt as we
paint the bike sheds our favorite color but we need to understand that
in a community that is supporting an open source package that not
everything will be accepted. New developers can join by being
participating as well as learning. Test out the new patches and give
> So I suggest to read a little about Sociocracy and start discussing our
> decision making process.
The decisions have been made. Now it is time to review, test, patch
again, re-review and retest. Find an interesting patch to review. Be
notified of a patch waiting for review by subscribing to the review tag
Make a Drupal difference and review core patches.
More information about the development