On Mon, 27 Jun 2016, Martin Winter wrote:
The difference is between a automated ‘proposed’ tree (no manual ACK)
What does this 'Ack' buy? It's just over 1 bit of information - except
that the lack of an Ack can mean either:
- People read it but didn't Ack it, cause
* Didn't really like it
* Have no opinion on it
* Like it, but didn't have time to Ack it then and/or forgot
- It slipped through the cracks
An Ack can mean:
- I read the patch and reviewed every line, and found no issues
- I glanced through the patch, and generally like the idea, though
I didn't really do a code review
- I read the message and liked it based just on that, I didn't read
further
- I like the person who submitted it!
If I'm integrating stuff into master, these 'Acks' are mildly
interesting but they're really not anything I base my decision on - as
per today. They don't convey much useful information.
So what problem is this supposed to achieve?
Further, if there's meant to be a group of people looking out for
patches, how is it clear who is responsible for tending to patches that
{day,week,month}?
There is _reason_ why I tried to introduce an explicit, unitary
'roundskeeper' hat. It is because one issue with the group approach is
that it isn't clear who should do the (boring) job of making sure every
patch is looked after in some way. It's easy - as one of a group of
integrators - to just look after the stuff you find interesting, and
avoid dealing with other stuff. Stuff falls through the cracks.
There are reasons.
and a tree with patches which have been ACK’ed by at least one other
person.
Your suggested way may work partially - but it’s still not clear on
how things move from that proposed to the master (I just use “master”
as the main development tree.
By a fast-forward merge. How would an Acked patch be committed?
I do NOT imply that this has to be the
master branch. I could be another branch, but “master” for development
seems to be common)
The name doesn't matter, but let's just call any central integration
nexus branch 'master', as it is now.
It would be less than the ‘proposed’ tree you suggest. But yes,
reverting or fixing previous commits might happen. Hopefully not too
many times if the people who send a ACK get used to do some decent
code review. But nothing stops us to modify the rule later if the
quality of the ACKs are not good enough.
So why not just have a staging branch? Like any many other perfectly
healthy projects have?
Also, at the same time anyone can send a NACK - which would stop the
patch to go in until the NACK is either resolved or (in the current
proposed maintainer doc) agreed by the maintainers.
This is all very manual. Manual processes are tedious and error-prone.
Why not get rid of this manual Ack + commit and automate that?
If you have to have manual processes, then they should be batchable.
Beside, your (or the current widely understood model) is that patches
go it if there is no NACK - so no active code review or any ACK
required. So this is nearly the same, just now REQUIRING an ACK.
A single NACK is still stopping the whole process until resolved or
overruled by some group (the maintainers in the current version) I
honestly fail to see how you think that this is making it worse. If
your only suggestion is to wait a min of 3, 7 or 14 days after an ACK,
then this can certainly be discussed.
I thought the development process _must_ speed up, but it seems the
proposal is to _add_ another manual step - on top of the existing ones?
:)
I want _fewer_ tedious manual steps in the integration process.
A good chunk of patches are uncontraversial fixes that should need
minimal actions to get in. Having to have people notice them on list,
hit reply and type "ack" (or whatever) sounds like needless extra
overhead?
What problem is this trying to solve?
Very few. I agree. I think this is a problem, you seem to be ok on it
(unless I misunderstand)
It's a huge problem. More people should take the time to review things.
Having contributors need to push others for a review will only work if
they spend some time reviewing patches from others as well. I think
that’s the beauty on this.
'Ack' doesn't convey review though.
Scaling issue at this time? Sorry, my feeling is that we don’t have
enough _full_ maintainers today with the time to be involved. Only you
seem to be regularly active (again).
The integration role, I had hoped to have had rotated around on a
monthly basis.
I think it needs a certain number of regularly active maintainers,
i.e. 3..5 minimum.
I don't think there's any issue quickly scaling out to 3..5 people
actively involved in integration.
Though, first, we have to agree.
No. But you made it clear that you have the final say (as a maintainer).
Where? Another person's objections are as valid as any others - least,
if they give somewhat plausible reasoning. Have you not noticed that I'm
still honouring NACKs from, e.g., David?
If what you were saying were true, there are at least 2 patches I'd just
go commit now.
And anyone sending a ACK doesn’t seem to trigger any action of the
patch moving forward - at least that’s the impression.
We were caught up (least in the forward direction) with patchwork by
November or so. When I last did an integration round.
What happened after that?
The next integration round, r8, should have us more or less forward
caught up, as well as nearly all the backlog dealt with.
There are various other groups who tried to push small (and large)
fixes into Quagga and got rejected.
What is wrong with rejecting patches?
But Cumulus did some really good enhancements and fixes to Quagga.
Their patches might not be perfect (i.e. cases for non-linux systems
like *BSD), but I think the good stuff fairly outweighs the not so
perfect or broken part by a wide margin.
Yes _absolutely_. I never said otherwise. Indeed, I'm sure I explicitly
said several times. You've quoted one instance even.
Nobody says that it needs to be released unchanged. By blocking based
on some non-perfect commits, we are blocking all the good stuff they
did at a later stage.
See, this is the *fundamental* issue. Who did the blocking?
I was integrating stuff *ALONG WITH* working through their backlog,
while I was doing the rounds.
*Donald* blocked the normal process with r7 on "all the take-3 should go
in".
With all the good stuff now missing, I personally would rather open
the floodgates, get their stuff in
Exactly what I'm doing. Except 'get their good stuff in' - and I'm still
going to push-back the stuff that has nits, problems or needs
discussion. Which is NOT that much. And they/we can figure out how to
address that too, and maybe some of it goes in later.
Cause _THATS HOW OPEN SOURCE SOFTWARE DEVELOPMENT SHOULD WORK_.
There are at least another choices: You could reconsider. I rather
have just one community and have all the parties working together.
This community isn’t just you. Yes, you’ve started Quagga and yes, you
own some of the essential resources (i.e. domain name), but there are
countless people which contributed or tried to contribute in the hope
to move this project forward. I do believe they should have a voice as
well.
When have I denied people a voice?
All I'm trying to do is point out some practical, reality based issues:
- on ACKs, that seems like it adds more overhead
- On having 2 branches, one for paul, one with voting so we can get
around paul, and releases from each, with no specified way of how they
stay coherent - that's just wanting to have a fork without having a
fork. We'll just end up with incompatibilities between them, and
confusion for distributors and users.
I think in such a case it'd be better for everyone to just properly
fork. We'll all be happier. We might even get on better.
My way or the highway seems to be your view… Sad.
What do you disagree with me on though?
You want stuff to be reviewed, but then you want a weird branch where
you can vote in a branch from one contributor with commits which one of
the most _active reviewers_ (I think I am anyway) has pointed out issues
of various kinds with or has other queries on? Some of which the
contributor has _ignored_ for nearly 2 years? Some of which are on
relatively recent patches, which have also been ignored?
Am _I_ really being unreasonable then in objecting to proposals to allow
contributors who've been ignoring comments to club together with others
(inc. some who don't realise that this is the driving issue) to vote and
override the comments?
I don't think I am being that unreasonable.
You’ve started a different way (introduced the proposed branches),
then handed it over to Donald, stepped back into inactivity, didn’t
ack him as full maintainer to allow making decisions and now blaming
him? Did I get this right?
I think he did an awesome job.
He was doing great, sure.
If you think I was being inactive, then you're unaware that I went
through the _entire_ take-3 branch in february or march and reviewed it.
Some of the issues I had, I even just provided the fixes for. We had
conference calls about one or two patches as well - e.g. to figure out
the UI issues of the triggered-route-map reprocessing patch.
It seemd to be going well. Though, after I'd given Donald my list of
review comments, he seemed to come back with arguments that perhaps the
branch should go in as is.
Not exactly sure what happened after that. There was probably some more
rebasing and practical code suggestions I could have done and sent him -
but there was also stuff I thought Donald / Cumulus was going to do
(e.g. look into the triggered-route-map reprocessing changes we'd
discussed and, I thought, agreed on).
I know Donald was going to have David do the next round (which I'd had
no issue with), and I presume there was some communication between them.
David posts to say r7 will be based on the take-3 branch - I point out I
had sent comments. Next thing is Donald is doing r7 as a bare minimum
bug-fix round, and there's this whole "development is broken! let's vote
through changes!" stuff.
So yeah, doing fine, still he stopped doing it. Apparently deliberately
because he wanted /all/ of take-3 in.
I have nothing against Donald personally, but "one branch for paul; one
branch to vote through massive branches, despite paul" doesn't work for
me.
I understand Donald is acting in Cumulus' interests. Don't hold it
against him. I don't think that was the best approach to take. I will
act in my interests so.
But he is only one person and he had to work on 2 branches (the public
and the internal cumulus). I would assume you understand the strain as
I assume you work currently on 2 branches
There was r6 too - screwy. Also, I believe Cumulus had a release at some
point.
In that case, get someone else to do the next round.
Also, integration work is very boring. Some of the motivating factors
for the 'roundskeeper' role are to try allow people to not get stuck for
too long with that, and be able to do other stuff too. Hence:
- Doing it in a batched way, so there's X time (e.g. 1 or 2 weeks) of
the boring, tedious, annoying patch-herding work; then Y time (maybe
also 1 to 2 weeks) of 'quiet' time for the integrator where /others/
do work (reviews) and the integrator can do something more
interesting, before doing the final collating based on review comments
to create the tentative 'accepted' - another little 'quiet' period
for testing, before final pushing.
- Rotating it around. If there's 3..5 others, then each potentially only
needs to do the intensive 'on roundskeeper duty' thing - that will
suck 2 to 4 weeks of time over the course of 4 to 6 weeks - once or
twice a year.
There are reasons for this stuff you know.
And he didn’t try to push the Cumulus branch in - at least not yet.
But I can see his frustration dealing with problems which he has long
fixed in the cumulus internal branch - but he is blocked by pushing
this into the community edition because some commits which you
disagree on.
And the answer to that is to block pretty much *everything*, until he
gets to be able to shove the whole thing in, inc. commmits I have
comments or queries on?
Again: Good chunk of it is perfectly fine to go in. No disagreement.
Hell, that good chunk would have been in _months_ ago, if we'd just done
this the sensible way and gone with the suggested rebase of take-3 for
round-7 + _everyone elses_ outstanding patches, and addressed the
comments on the remainder for the next or later round.
That'd have been much better, but hey.
Thanks for the summary, but to be honest, I thought we are talking
about governance, not actual patches.
Except, that's the driving factor behind why Donald stopped integrating
patches generally (except for a weird minimal bugfix-only r7).
Tease out the strands of the problem and address them one by one - often
the best way to go.
All the proposed scenarios allow to NACK patches and discuss them. The
current proposed maintainer even requires a ACK by a 2rd party for it
to move forward.
This sounds slower, more manual, and more bureaucratic than what we have
today!
What may change is (in the current proposed maintainer doc) that you
can’t VETO it anymore, but the maintainers together in a collective
can still decide against it if any NACK (from maintainer or not) is
not resolved first.
See, I think this is a crappy way to resolve the Cumulus backlog issue.
I don't see how this is a reasonable way to tackle that. Indeed, this is
a _huge_ diversion of time and energy from what is required:
Work through the backlog.
There are 2 issues here:
1) Backlog
How to deal with Backlog may require a special exception. At least I
can’t think about any reasonable way to review the large amount of the
backlog just from Cumulus alone.
I *DID REVIEW IT* _months_ ago.
Indeed, some of the patches I reviewed YEARS AGO!
Do you see the issue?
2) New submissions
If we can’t speed up the review cycle, then this situation will happen
again and again. The maintainer (and tools and community) doc try to
address these shortcomings.
I'm not convinced those involved understand the issues, least they don't
seem aware of even the very recent past. You can't tackle shortcomings
without that.
E.g., how does _adding _manual steps speed things up????
- As far as I know, at no stage does Donald communicate to others that
the nub of the issue is he wanted take-3 to go in without further
discussion and without addressing my comments - some of which
Cumulus
have had nearly *years* to act on.
You seem to like picking on Donald. He is only one of the community
members which would like to fix the process.
I'm "picking on" Donald because he is the one who started this process
of agitating for majority votes, because he (or others in Cumulus) want
to have take-3 shoved in as is - and they don't want to have to deal
with community reviews (some outstanding for over a year).
That's what has driven us to here. I simply can not fail to mention the
facts of this, because this is the _prime point of disagreement_ between
us. And Donald has _not_ communicated this to others in this process, as
far as I know.
I could say "Cumulus" instead of "Donald".
Nothing personal against Donald.
My impression was that everyone (except you) wants change.
Funnily enough, when I explain the prime point of disagreement, Lou and
Jafar seemed to indicate it should _not_ be the case that contributors
get to rewarded for ignoring comments. Least in principle.
However, it speeds up sorting the differences over the slower email.
So I think this helps to get to a agreeable solution.
Sure, you can use it to augment and help form. That's fine.
Why work with people you find troublesome? I honestly don't want anyone
who thinks that of me to have to suffer further so.
Again, you are saying that you are unwilling to discuss any changes… Sad.
I didn't say _any_ changes. Indeed, some of the stuff in the document
may be perfectly agreeable. What changes do you want to make?
However, voting through branches cause the contributor has ignored
comments for long they find it too bothersome to address any?
Having two branches with a weird status to each other?
regards,
--
Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A
Fortune:
Aberdeen was so small that when the family with the car went
on vacation, the gas station and drive-in theatre had to close.
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
