[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Gnu-arch-users] what's all the fuss about merge requests?
From: |
Tom Lord |
Subject: |
[Gnu-arch-users] what's all the fuss about merge requests? |
Date: |
Tue, 6 Jul 2004 23:43:41 -0700 (PDT) |
Here is a message that illustrates some of why we're talking about
merge request formats these days. Various names left off since this
is (less than, even) a draft and the discussion and work are still
ongoing.
Date: Fri, 2 Jul 2004 20:37:45 -0700 (PDT)
From: Tom Lord <address@hidden>
To: address@hidden
CC: xxxxxx, xxxxxx, address@hidden, xxxxxx
Subject: Re: tla stable branch
Ok.... so --- revising based on your feedback, here's a revised set of
rules.
I've added a friend of mine (and the project's) to the CC list who I
would like to keep informed about this discussion.
The Arch Project Co-Maintainer Merge Approval Process
by maintainers:
xxxxxx
James Blackwell
xxxxxx
Tom Lord (Chief Architect, document author)
Andrew Suffield
Bug Goo gives us a pool of pending merge requests. I assume that
votes are recorded there? When an issue receives a vote that
determines the fate of the bug, it's either queued for merging or
rejected.
I think that the system needs a combination of "votes" and
"directives". Votes are roll-calls of maintainers voting in various
ways (most recent vote per maintainer counts). Directives can trigger
actions that the voting system makes only optional rather than
deterministic.
One example of a directive is a redirection of the merge source.
Suppose that a patch arrives. It's almost right and I want to fix it
rather than kick it back for replay. I should be able to redirect the
merge source.
I think we want to offer users a definate service-level guarantee:
they get a reply for the patch in 72 hours. The reply can be any
of the following things:
~ accepted
Your merge went through, perhaps with revisions.
~ rejected
We are not interested in this change at all.
~ returned
We are interested in this but will not merge it
right now. You're encouraged to make the changes
suggested in the Bug Goo discussion (especially the
messages containing votes) and resubmit, no sooner than
48 hours from now (unless the maintainers have said
otherwise).
~ confused,holding
We are confused about your patch and haven't come to
a resolution yet, however, for now we're holding on
to it. You should join the mailing list for the
issue [*] if you want to help resolve the confusion.
~ confused,returned
We are cnofused about your patch and haven't come to a
resolution yet. We're returning it to you with our
various comments attached. If you would like to try
again, please wait 48 hours unless the maintainers
have said otherwise.
To a first approximation, we're all equals and we're interested in
pair programming. We're all conservative about changes to arch. We
all want to give excellent turnaround time and a friendly face to each
other and to other contributors.
Putting those generalities together, I think that "by default", we
want a rule that if two of us agree about something, and there is no
substantial disagreement from others, then whatever those two agree on
happens. However, it's better to err by not merging than by merging
(since we're conservative) and it's better to err by returning or
expressing confusion than to err by rejecting (since we want to to be
efficient and friendly).
A straw-man first idea is that we can each vote:
accept
reject
return
and if any of those categories wins by N>=2 to 0, the pair programming
rule is satisfied, and the result is determined.
Strict pair programming will never work. If we leave it at that, we
will devalue the meaning of all three kinds of votes. Those three
possible votes, "accept", "reject", "return" should ideally mean "I
have looked at this carefully and I say we should ____".
What will really happen, if we leave it at that, is that we'll all
start cheating. I'll say "Hey, Andrew, I just submitted 5 changes
and approved them. Trust me, they're fine, please approve them."
And he'll think to himself "Yeah, right." and look at them and decide
for himself -- but even so, when he looks at them, he'll be biased.
He'll think it probable that I'm telling the truth and that my
judgement is right. He _won't_ look at them all that carefully.
I happen to think that that dynamic is fine --- it isn't bad that
the maintainer's will cut corners that way. What would be bad is for
it to happen by accident.
After months of that corner-cutting, it will be habitual. It won't
have to be negotiated in each case. People will just "know" when to
go ahead and approve something for someone else. And it'll start
happening that some things get "casually approved" by two people and
"carefully approved by none". Which violates our conservativism.
So, I want to make the short-cut official and safe. I want people
to be able to say "I'll approve, but the other approver should look at
this more carefully than I did."
So, the next straw-man idea is the votes:
accept
weak-accept
return
weak-return
reject
weak-reject
so that these are clear votes:
results:
accept accept return return reject reject
vote counts:
accept >=2 1 0 0 0 0
weak-accept d/c >= 1 0 0 0 0
return 0 0 >=2 1 0 0
weak-return 0 0 d/c >= 1 0 0
reject 0 0 0 0 >=2 1
weak-reject 0 0 0 0 d/c >=1
That strawman has a forseeable bug, too. Some maintainers might vote
weakly on a patch followed by others voting unamimously and stongly.
It shouldn't always be necessary to get a weak voter to retract his
vote. On the other hand, lots of dissenting but weak votes does
suggest that something is up and human attention is required. So, I
would balance those with the third strawman:
results:
accept return reject
vote counts: | |
accept >=2 >=2 >=1 | 0 0 0 | 0 0 0
weak-accept d/c d/c >=1 | <=1 0 0 | <=1 0 0
return 0 0 0 | >=2 >=2 >=1 | 0 0 0
weak-return <=1 0 0 | d/c d/c >=1 | 0 <=1 0
reject 0 0 0 0 0 0 >=2 >=2 >=1
weak-reject 0 <=1 0 0 <=1 0 d/c d/c >=1
which makes the voting system immune to up to one stray weak-* vote.
Sometimes patches will arrive that many would agree need my review.
Perhaps sometimes there will be disagreements among maintainers and
someone will think "Screw it: this is up to Tom". One choice is to
handle that all informally: just send me email. But that's
unreliable. I might not have time to reply just then. That kind of
email can get dropped. It's better to make "get Tom" a persistent
part of the voting system. So, straw-man 4 adds the possibility of
voting "flag":
results:
accept return reject
vote counts: | |
accept >=2 >=2 >=1 | 0 0 0 | 0 0 0
weak-accept d/c d/c >=1 | <=1 0 0 | <=1 0 0
return 0 0 0 | >=2 >=2 >=1 | 0 0 0
weak-return <=1 0 0 | d/c d/c >=1 | 0 <=1 0
reject 0 0 0 0 0 0 >=2 >=2 >=1
weak-reject 0 <=1 0 0 <=1 0 d/c d/c >=1
flag 0 0 0 0 0 0 0 0 0
After responding to a flag, it shouldn't be necessary for the flag
raisers to explicitly lower the flags. At the same time, I should be
free to reply in some way other than voting a (too be added later)
"trump" vote. For example, I might want to clear a flag raised
because "I'm not sure this option name is right" and I might want to
reply "the option name is fine although I haven't reviewed the patch
beyond that".
Therefore, I'd like to ask for a chief-architect-only _directive_ (not
vote) by which I can convert either a specific or all "flag" votes
into non-votes.
Speaking of my special powers: please indulge me by giving me trump
cards. That way I can just "do stuff" either to clear outstanding
deadlocks or because I'm really sure it's necessary. (For example:
perhaps the FSF wants me to quickly update the copyright notices in
some way.)
So: strawman 5 adds the "trump" votes, available only to the c.a.:
results:
accept return reject
vote counts: | | |
trump-accept 1 0 0 0 | 1 0 0 0 | 1 0 0 0 |
trump-return 0 0 0 0 | 0 0 0 0 | 0 0 0 0 |
trump-reject 0 0 0 0 | 0 0 0 0 | 0 0 0 0 |
accept d/c >=2 >=2 >=1 | d/c 0 0 0 | d/c 0 0 0 |
weak-accept d/c d/c d/c >=1 | d/c <=1 0 0 | d/c 0 <=1 0 |
return d/c 0 0 0 | d/c >=2 >=2 >=1 | d/c 0 0 0 |
weak-return d/c <=1 0 0 | d/c d/c d/c >=1 | d/c <=1 0 0 |
reject d/c 0 0 0 d/c 0 0 0 d/c >=2 >=2 >=1
weak-reject d/c 0 <=1 0 d/c 0 <=1 0 d/c d/c d/c >=1
flag d/c 0 0 0 d/c 0 0 0 d/c 0 0 0
Again, given considerations such my historical relation to the project
and, perhaps more importantly, my obligations to the GNU project,
I will need some directives:
freeze --- only c.a. trump votes have effect for now
thaw --- back to normal
I don't personally have an immediate need for "slush" mode (whatever
that might mean) but it might be good to keep in mind.
The above rules give us conservativism. I think they are likely to
give us good throughput among ourselves. What about good throughput
and friendly face?
The goal was to give an informative reply for every patch w/in 72
hours. What the voting system strawmen still don't solve is what do
if 72 hours pass without a conclusive vote.
Gratuitously kicking back an unresolved patch to someone only to have
them resubmit is not a friendly face. By default, we should hold
onto most confusing patches, at least for a little while, and try to
resolve them.
On the other hand, sometimes we'll generally agree about an unresolved
patch that no resolution is worth working on and so, kick it back
labeled "confused" to the submitter.
Some of that can be automated. So, the next thing to add to the
voting system, in addition to the above rules, is:
If the table for accept/return/reject doesn't
apply after 72 hours then:
results:
confused-hold confused-return
vote counts:
trump-accept 0 0 0
trump-return 0 0 0
trump-reject 0 0 0
accept >=1 d/c d/c
weak-accept d/c >=2 d/c otherwise
return d/c d/c d/c
weak-return d/c d/c d/c
reject d/c d/c d/c
weak-reject d/c d/c d/c
flag d/c d/c 1
Finally, what happens _after_ 72 hours to "confused-hold" patches?
Just a periodic email reminder to maintainers and submitters is _ok_
except that, as noted in previous discussion, we want two levels
of "begging off" from such reminders, hence two more possible votes
(for which all of the columns in the tables above have "d/c") are:
unable
Meaning: "I'm not voting on this one and never will."
abstain
Meaning: "I'd rather not have to vote on this one."
When the periodic pass over older-than-72-hour patches happens:
1. If nobody has not voted, all "abstain" votes are converted
to non-votes.
2. If, after that, everyone has still voted (there were no
abstains) then all "unable" votes are converted to non-votes.
3. Finally, all non-"unable" maintainers are reminded about the
patch.
Next topic: automatic testing.
Asynchronously with merging, a separate process repeatedly runs the
tests on the mainline. It maintains a tag-only branch with the
outcomes of each test result.
The latest revision in tag-only is always of the latest
revision that passes the test suite. There may be gaps:
stable tests
patch-a -----------> patch-b
patch-a+1 ,----> patch-b+1
patch-a+2 /
patch-a+3 ----'
As a final rule for the voting system, a final modification to the
rules above:
If the latest revision in stable has not passed for at least 24 hours
then the patch manager is said to be in "regression mode".
Two additional possible votes are available:
fix-regression
weak-fix-regression
These normally behave like "approve" and "weak-approve".
When the patch manager is in regression mode, the usual
rules apply with the qualification that, to be merged,
a patch must have either a "trump-approve" vote or at least
one "fix-regression" vote.
And, as a final directive, any maintainer should be able to send
the directive:
test $patch
and receieve in return mail (and on a web page) the result of a test
run of a recent mainline revision, with $patch applied. The
directive:
test-approve $patch
should run and return tests and, if the tests are a clear pass,
automatically register an "approve" vote for the sender.
-t
- Re: [Gnu-arch-users] Arch Roadmap Draft (the anticipated part 3), (continued)
- Re: [Gnu-arch-users] Arch Roadmap Draft (the anticipated part 3), James Blackwell, 2004/07/07
- [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), Miles Bader, 2004/07/07
- Re: [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), James Blackwell, 2004/07/07
- [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), Miles Bader, 2004/07/07
- Re: [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), James Blackwell, 2004/07/07
- Re: [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), Tom Lord, 2004/07/07
- Re: [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), James Blackwell, 2004/07/07
- peace rocks Re: [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), Tom Lord, 2004/07/07
- against paranoia Re: [Gnu-arch-users] Arch Roadmap Draft (the anticipated part 3), Tom Lord, 2004/07/07
- Re: against paranoia Re: [Gnu-arch-users] Arch Roadmap Draft (the anticipated part 3), Stephen J. Turnbull, 2004/07/07
- [Gnu-arch-users] what's all the fuss about merge requests?,
Tom Lord <=
- [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), Miles Bader, 2004/07/07
- Re: [Gnu-arch-users] Re: Arch Roadmap Draft (the anticipated part 3), James Blackwell, 2004/07/07
Re: [Gnu-arch-users] Arch Roadmap Draft (the anticipated part 3), mlh, 2004/07/06