[development] Reviewing patches and making decisions -> Sociocracy could be a way to go!

[Earnie Boyd]

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 
an issue.

> Do you agree that these information could help a lot in similar situaltions?
>

Not always.

>
>> 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 
> community
> 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 
review.

>
> 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 
at r-feed.com.

--
Earnie
   http://r-feed.com
   Make a Drupal difference and review core patches.