[Python-Dev] discourage patch reviews to the list?
"Martin v. Löwis"
martin at v.loewis.de
Thu Feb 10 00:10:01 CET 2005
Brett C. wrote:
> But if people don't have that in mind, should we not be encouraging
> this? I mean it seems to be defeating the purpose of SF and having the
> various mailing lists that send out updates on SF posts.
Clearly, the comment should *also* go to SF - posting it to python-dev
may mean it gets lost eventually (in particular, when somebody gets to
look at the patch).
Björn did post his comment to SF, and a summary to python-dev. I
personally think this is a good strategy: it puts focus on things
that should be worked on.
Let me explain why I think that these patches should be worked on:
- it might be that the analysis of the patch suggests that the patch
should be rejected, as-is. If so, it has a good chance to be
closed *right away* with somebody with write privileges to the
tracker, if he agrees with the analysis taken. People who care
can follow the link in the email message, and see that the patch
was closed. People who don't care can quickly grasp this is a patch
review, and delete the message.
- it might be that the analysis suggests changes. Posting it to
python-dev gives the submitter of the patch a chance to challenge
the review. If somebody thinks the requested changes are unecessary,
they will comment. People actually prefer to discuss questionable
requests for changes on the mailing list, instead of discussing
them in the SF tracker.
- it might be that the analysis recommend acceptance. Again, it might
be that this can trigger a quick action by some committer - anybody
else can safely ignore the message. However, *some* committer should
take *some* action on the patch - one day or the other. Having
the right to commit is a privilege, but it is also an obligation.
The patch needs to be eventually looked at, and decided upon.
Somebody already did the majority of the work, and suggested an
action. It should be easy to decide whether this action is
agreeable or not (unless the review is flawed, in which case
the reviewer should be told about this).
To put it the other way 'round: should we only discuss changes on
python-dev which *don't* have patches on SF???? I don't think
Furthermore, this strategy exposes the reviewer. A reviewer is
somebody who will potentially get write access to the tracker,
and perhaps CVS write access. A reviewer who wants to contribute
in this way regularly clearly needs to gain the trust of other
contributors, and posting smart, valuable, objective, balanced
reviews on contributed patches is an excellent way to gain such
trust (likewise, posting reviews which turn out to be flawed
is a way to find out that the reviewer still needs to learn
things before he can be trusted).
P.S. These remarks are mostly of general nature - I haven't
actually studied yet Björn's review (but I leave it in my
inbox so I can get back to it next week).
More information about the Python-Dev