Hi Peter,
I want to apologize if it seemed that I asked you to hold back on any
effort, especially some not directly related to day-to-day tasks, I
did not mean anything like that. [...]
That said, of course it would be great if more developers try the
new submit strategy and see how they feel about it.
No worries, that's not how I interpreted your emails. Like Martin said,
it would be good if more developers with submit rights would try the new
submit strategy and I'd also appreciate if more submitters would
regularly go through the submittable patches, do another quick review
and if it's all good submit the patches; otherwise request some change.
I don't mind doing this regularly, but it's not too great if submittable
patches will just sit there for a week or two when I'm out of office or
too busy with work. This has already been the case a few times.
Nod, that's very understandable. For this, do you think it could work to
not push everything at once, but only one "topic" at a time? [...]
The workflow would then be to push the first "topic" (short patch train)
to Gerrit, have that enter master, pull and then rebase the following
local branches onto master, push next "topic" and so on.
This has both benefits and drawbacks; a benefit is that there are
more frequent but much smaller patch sets to review, a drawback is
that each patch set requires one "Gerrit turnaround" which for a
large project could take a while.
The drawback of more Gerrit 24h turnarounds is at least to me a very
significant one. The 24h turnaround is a good compromise between still
keeping things moving relatively fast and giving more developers a
chance to review things, so I consider this to be a good thing. It is
however necessary to work with this in a way that doesn't slow down the
development and upstreaming too much.
When upstreaming some longer patch train or directly developing new SoC
support on upstream, I try to not have more than 2-3 topics in one patch
train, but that allows me to upstream things at a similar rate as I
write code and the upstreaming won't stall the development. When the
upstreaming is far ahead of the development an additional problem would
be that some requested changes in the review might affect all local
patches that aren't in review yet which makes it more difficult to
address requests in the review. Also when developing directly on
upstream, I don't want to hold back too many patches locally to have a
backup of what I already wrote.
the submit together with previous patches button on the last
submittable patch in the train should be used to not need to rebase
most of the patches after the previous patch got submittet, the rebases
might not be as much of a problem than I initially thought.
That's a good point - but that only applies to the "perfect patch set"
case, right? It's cool if we can optimize that but I wouldn't want to
make things a lot worse for patch sets needing some turnarounds.
I'd guess that this only applies to the cases where the full patch train
has been pushed and not only partial repushes after fixes. Haven't
verified this though.
In cases where the rebases are mostly trivial and only the surroundings
of the added or changed code have changed due to another patch train
being submitted, I probably wouldn't consider the rebase a change that
would require restarting the 24h cool-down time, but for a non-trivial
rebase, I'd likely wait another 24h after the non-trivial rebase. I try
to push things forward quite a bit, but still don't want to do this at
the expense of possibly missing bugs that could have been caught.
What if Jenkins does not build until human has given +2 ?
That would make it much more difficult to see what should be reviewed.
Right now a Jenkins +1 is a good indicator for me that the code is ready
to be reviewed; if Jenkins doesn't like a patch, I don't think that a
reviewer should already spend much time on already reviewing it. So at
least for me the Jenkins +1 is rather important information before I
start reviewing patches.
Regards,
Felix
_______________________________________________
coreboot mailing list -- [email protected]
To unsubscribe send an email to [email protected]