On 23.10.2017 09:39, Stefan Tauner wrote:
On Fri, 13 Oct 2017 17:42:11 +0200 Nico Huber nico.h@gmx.de wrote:
What I didn't realize last night: the `staging` branch contains valuable information in lots of fixup! commits that would be lost if we don't keep `staging`. They look ugly in the log but their messages still contain some reasoning about the changes. I prefer to use the current `staging` branch as `master` therefore.
*If* there is valuable information in them then they should get into the final commit message (i.e. the one that squashes everything to stable). I deem them basically as part of the review process of the whole change (i.e. the commit eventually committed to stable) and sure, there is information in it that might be interesting but this likewise applies to all review comments and I would not want them to be part of the commit messages, would you?
No, ofc not. But there is always a compromise of what to include into the commit message. Though, maybe I'd write too elaborate ones, I think yours are often too thin (it only doesn't look like it because you do too much in single commits, IMHO).
I have to admit, it's not a strong argument. But I thought, if I ask which branch to follow, I should vote for one too ;)
I actually don't care much about which branch we'll choose (current staging) or a history rewrite of it. BUT, this branch should be used for future development with appropriate write permissions for any major flashrom contributor (having few submitters just doesn't scale).
Some arguments why using staging for anything than hacking is not a good idea:
- The information on a change is spread widely.
In your notion of a change, this was and will always be true. Most commits aren't perfect and may result in future changes.
For someone not involved in the respective review/development process (i.e. everybody but the author of the fixups and respective reviewer(s)) it is very hard to track what the actual outcome of all of the fixups is.
In the git way of fixups (e.g. `fixup! <original commit>`), this is pretty easy: The outcome is what the original commit message promised.
Having a single commit that has a decent commit message solves this issue.
s/decent/perfect/?
I'd prefer decent commits and messages, too. But not if that means we reduce the rate to one commit per year. And I don't see why the tran- sition to git should be the starting point to only allow perfect com- mits. If I see something weird in flashrom code and look the commit up, it's often one that does multiple things at once and only lists what changed and not *why*. What we pushed to staging so far (including the weird looking fixups) is already more useful than commit messages that only rephrase the patch hunks.
- The log of a branch with lots of interwined fixups as a whole does not only look ugly, it does also no longer serve its purpose well: reading through it should give a good overview of what the code does.
You seem to imply perfect commits that only add new code. But in fact most commits change code. A decent commit message explains why the change was necessary and maybe why it's correct (if that is not ob- vious). For new features I agree, it should state "what the code does".
- The staging branch does currently and probably will continue to have some build problems, and due to the current rate of merged changes contain a significant number of undetected run-time bugs as well.
Feel free to compare regression rate before and after `staging`. I'm still convinced, we fixed more regressions than we have introduced so far.
Both can make bisecting for specific problems tedious if unlucky.
Lol, that's still better than squashing commits into bigger ones which makes bisecting less feasible.
Nico