Felix, If you've formed an opinion, maybe you want to take a break from submitting for a bit to let others test out the new submission strategy?
For everyone else, I'd like to note that over the past year, Felix has hit submit on more than 45% of all of the coreboot patches that have been merged. The next highest submitter merged just over 10% and the person after that merged 5.5%. I'd weigh Felix's opinions on this matter very strongly.
If we continue with this method (and for the rest of this month), I think we want to encourage shorter patch trains, consisting of only related patches. Currently, it seems like projects will occasionally build up enormous trains of patches that aren't necessarily related so that they can pull all of their patches at once just by grabbing the last one.
It also sounds like rebasing all of the patches so that the entire train is contiguous is really needed - no more patches based on older versions of the previous patch. I think the same is true for abandoned patches - rebase everything so that the patch train isn't broken in the middle. Martin Aug 2, 2022, 14:02 by felix-coreboot@felixheld.de:
Hi Peter,
By "keep an overview" I mean to know which commits in a pushed branch that have been reviewed and which not.
Ah, ok; I'd assume that Gerrit won't allow pushing a series of patches if one patch in there doesn't have +2 and +1 verified yet. Haven't verified this though.
I'm all for skipping unneeded build load [...] What am I missing here?
Let's assume that we have a patch train with 10 patches and during the review a typo fix in the first patch is requested and all other patches got +2s without any change requests. Until now the solution was to update the first patch and only push the first patch with the typo fix and just leave the following 9 patches at their initial version, so Jenkins won't build those 9 patches again. When re-pushing the full patch train, the other 9 patches will get build tested again after the unnecessary update and again when the patches get submitted. I don't think that this is a huge problem, but still something to think about if we want to change this from what we've done in the recent past.
I reread the documentation and the thing I missed in there is that Gerrit will only automatically rebase a patch when submitting when none of the files it changed have been changed between the parent commit of the to be submitted patch and the current top of tree.
That seems like an improvement to me. By the way, I don't think Gerrit has ever actually rebased anything, nor will it with the new setting, I understand it to always do an equivalent of "git cherry-pick" but with different requirements with the new strategy.
Rebases have always been manual, but before it didn't matter too much, since the cherry-pick strategy just tries to cherry-pick a commit and only requires a manual rebase when there's a git conflict that makes the cherry-pick fail. Now it requires a manual rebase when any of the files the commit changes have been changed in the meantime and not only when there are git conflicts, which happens way more often; especially when there are tree-wide changes or in the case of patch trains where the patches change the same files; see my next answer on the latter.
Hmm, how so? Submitting one commit at a time should make top of tree always match the parent commit of the to-be-submitted commit, right?
That was my assumption, but that turned out to not be the case which was the main reason for my first email today. Gerrit adds some lines to the commit message which will change the git hash; maybe that's part of the reason?
There was a patch train with at least the first few commits being submittable and after i submitted the first patch, Gerrit now shows merge conflicts on all following patches that should still be mergeable which is a dealbreaker for me. If you want to have a look yourself: CB:66272 was the first patch i submitted and now CB:66273 and following suddenly have merge conflicts which they didn't have before.
Regards, Felix _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org