[haiku-development] Re: Change patches workflow [was Final Set*UIColor Patch, Version 3e]

  • From: Adrien Destugues <pulkomandy@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Tue, 1 Dec 2015 20:53:00 +0100

On Tue, Dec 01, 2015 at 11:24:01AM -0600, looncraz wrote:

10. Commit to experimental branch
11. Experimental branch patches moved to master after community testing
This means you expect "the community" to run the experimental branch.
Two possible outcomes:
* The master branch is useless, because everyone runs experimental (same
thing we have with our 3-year-old release currently). Everyone expects
the experimental branch to be stable and complain when it isn't, or:
* Everyone uses the master branch, and 50% of bugreports get closed with
"already fixed in the experimental branch".

No, the experimental branch is a staging area for new commits. The master
branch will have all patches that are in exp that have been shown to be
stable, functional, safe, etc. added on a regular basis. However, you will
certainly have a few times where bugs are fixed/existing in one branch and
not the other. That's a minor issue, even more so when you have a
shorter-lived testing cycle.

There is also the problem that merging stuff from "exp" to "master" must
be done by someone, unless we can automate it. We did this for the alpha
branches, and for being release manager for one of them I can tell you
it is a LOT of work to keep the branch up to date.

It also means there must be some kind of decision process for when to
merge something in master. You listed some things to check, but someone
has to actually check them and make the decision to merge or not merge.

So, this may make the process a bit smoother for patch submitters, but
only as long as the reviewers can keep up. Which probably wouldn't be a
very long time, given how much work the review side is, and our
relatively small development team.


For example, if my color updating patch was applied to exp right now I'd be
getting feedback about any possible issues that could never be discovered in
review. People who are interested in staying to the bleeding edge will use
nightly/exp, those who care more about a stable, clean, environment will use
nightly/stable This seems pretty par for the course.

Style fixes are nearly LAST, but glaring issues are pointed out when
noticed, which will help eliminate the duplication of effort and submitter
frustration.
You can work any way you like. You can post small patches whenever you
have something working and ask for early review. Expect people to point
out both style and design issues - why should they ignore style issues
as they spot them?

I specifically stated that glaring issues should be pointed out when
noticed, but minor issues such as missing spaces, unimportant typos, etc.,
should be ignored. There's nothing more annoying than going through a file
many times repairing minor styling issues and then, days later, being told
to just delete the file outright, or to completely implement it another way.

Take it as training to learn the code style. That the code is thrown
away is irrelevant, the important part is you learn about all the
subtleties of the coding style until you get it right and automatically
on the first try (or get so annoyed that you write tools to ckeck behind
yourself). This is one step towards getting commit access. And yes, I
think even the minor problems with spacing and the like are important.


As a submitter, try to find a good balance: submitting a patch too often
will make the devs think "oh no, not again", especially if it doesn't
include all the feedback from the previous run. Not submitting often
enough, you risk your next iteration to be in a wrong direction in a
fundamental way, which means major rework for you and frustration on
your side.

Submission should be an interactive process. This is the hand-holding you
need to do if you want to get more quality submissions. As a submitter
gains experience in this process, the need for review declines and the
process becomes:

1. Submit proposal
2. Feedback
3. Build patch
4. Review
5. Live testing in exp branch
6. Application to master/stable after any repairs performed

Ok. I still think Gerrit or some similar tool would provide a similar
workflow, but more streamlined and automated, resulting in less work and
frustration for both the submitters and reviewers.

The main difference would be that there isn't a single "exp" branch, but
commits staged for review in gerrit can be cherry-picked for review and
testing. I used it (as a patch submitter) on another project and I think
the approach is quite nice.

--
Adrien.

Other related posts:

  1. Home
  2. haiku-development
  3. Archive
  4. 12-2015