Обсуждение: HEAD is open for 8.5 development
Let the breakage begin ... regards, tom lane
On Wed, Jul 1, 2009 at 7:30 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: > Let the breakage begin ... > > regards, tom lane Tom - and all other committers - In order to avoid duplication of effort, we should avoid assigning round-robin reviewers to any patches which the committers feel that they already have adequately covered. In order to do that, we need to make sure we know which ones they are. If you are taking a look at any of the patches submitted for this CommitFest, please put your name next to them on the wiki. Please ALSO add a comment saying something like "tgl says: I am reviewing this, no need to assign a round-robin reviewer". During the last CommitFest, we had occasional shenanigans where someone would stick the name of a committer or other community member next to a patch just because they had made some comments on it. But the person whose name got stuck there didn't necessarily know about it or feel any responsibility to conduct a complete review. So it would be helpful if everyone could make sure to add a comment when claiming patches. Thanks, ...Robert
On Thursday 02 July 2009 05:29:58 Robert Haas wrote: > If you are taking a look at > any of the patches submitted for this CommitFest, please put your name > next to them on the wiki. Please ALSO add a comment saying something > like "tgl says: I am reviewing this, no need to assign a round-robin > reviewer". > During the last CommitFest, we had occasional shenanigans where > someone would stick the name of a committer or other community member > next to a patch just because they had made some comments on it. But > the person whose name got stuck there didn't necessarily know about it > or feel any responsibility to conduct a complete review. So it would > be helpful if everyone could make sure to add a comment when claiming > patches. Um, how about no one sticks no one's name to any patch unless it is themselves or they are the commit fest management team and the reviewer has explicitly OK'ed it. Let's not begin to use several levels of "Reviewer: X", "X says: I will really review it.", "RH says: This X is legit.", "Reconfirmed-by: X". The wiki is weird enough to use that we shouldn't need to edit it in more than one place for each logical step.
On Thu, Jul 2, 2009 at 2:42 AM, Peter Eisentraut<peter_e@gmx.net> wrote: > On Thursday 02 July 2009 05:29:58 Robert Haas wrote: >> If you are taking a look at >> any of the patches submitted for this CommitFest, please put your name >> next to them on the wiki. Please ALSO add a comment saying something >> like "tgl says: I am reviewing this, no need to assign a round-robin >> reviewer". > >> During the last CommitFest, we had occasional shenanigans where >> someone would stick the name of a committer or other community member >> next to a patch just because they had made some comments on it. But >> the person whose name got stuck there didn't necessarily know about it >> or feel any responsibility to conduct a complete review. So it would >> be helpful if everyone could make sure to add a comment when claiming >> patches. > > Um, how about no one sticks no one's name to any patch unless it is themselves > or they are the commit fest management team and the reviewer has explicitly > OK'ed it. I'm all in favor of that. > Let's not begin to use several levels of "Reviewer: X", "X says: I will really > review it.", "RH says: This X is legit.", "Reconfirmed-by: X". The wiki is > weird enough to use that we shouldn't need to edit it in more than one place > for each logical step. Well, IIRC, Tom was very insistent when we first set this up that the last column of the chart should be labelled "Reviewers" rather than "Reviewer", so as not to project the idea that if there already is a reviewer, then no other reviewers need apply. And in fact we do sometimes assign multiple reviewers, if the first one gets busy and can't continue reviewing or if the first one flakes out and never posts a review at all or if the patch needs review from somebody else with a different skillset or if someone who has not volunteered to be a rrreviewer takes an interest in a patch and volunteers to review that specific thing or if (ha ha ha) we have enough rrreviewers to assign extras to certain patches. So I think it's important to clarify the distinction between when a committer is planning to look at the patch but maybe somebody else should too, and when the committer is planning to take exclusive responsibility for the patch and nobody else should bother touching it. Unless, of course, we want to make the rule that if a committer's name is next to a patch, then that automatically means that no help from anyone else is needed. But that seems like a policy that would inevitably give rise to exceptions. Also, in general, I have found that when trying to recall the status of a patch, the comment trail is a lot more useful than the main summary line. It's a lot easier to look at the comment trail and try to remember whether anything has happened "since then" than it is to look at the at the summary line and try to remember whether it matches current reality. Also, if people date-stamp their comments (which will happen automatically if we get over to my new system), it makes it possible to get a sense of how long it's been since a certain state change took place. Right now, I can look at the Wiki page and say, "Oh, Peter is looking at a bunch of patches, great". And I know that it's recent because your name wasn't there a couple of days ago, and I don't really care whether you're going to commit them soon because the CommitFest hasn't even started yet. But by the first of August I suspect there will be so much activity that without a comment trail it will be impossible to reconstruct what is going on. So I'd like to encourage not just committers claiming patches but ANYONE who updates ANYTHING to leave a comment with the details. ....Robert
On Thu, 2 Jul 2009, Robert Haas wrote: > I think it's important to clarify the distinction between when a > committer is planning to look at the patch but maybe somebody else > should too, and when the committer is planning to take exclusive > responsibility for the patch and nobody else should bother touching it. Suggesting additional work for the committers is never a good idea, and the tagging scheme you suggested seems pretty heavy for something that's not particularly common. How about you add an additional "Committer" field to the table, to be filled in when it's appropriate to do so and defaulting to "nobody" as usual. That would be handy in a lot of cases to track who is working on that part of patch application anyway. And in the special case you're trying to handle, I'd think listing their name under both reviewer/committer fields would be sufficient to deflect wasted review effort. > Also, if people date-stamp their comments (which will happen > automatically if we get over to my new system), it makes it possible to > get a sense of how long it's been since a certain state change took > place. The Mediawiki standard here is that you can use the various tilde macros to fill these in: http://www.mediawiki.org/wiki/Help:Signatures I wonder if it's possible to include that expansion in the CommitFest templates? Even if it's not, we could strongly encourage its use. When you're using the page editor, one of the little buttons at the top is "Your signature with timestamp", and it expands to "--~~~~"; that makes it easy to remember this particular bit. -- * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD