[U-Boot] [PATCH ARM 1/3] s3c24x0 code style changes

[Scott Wood]

On Tue, Dec 15, 2009 at 10:51:14PM +0100, Wolfgang Denk wrote:
> Dear Scott Wood,
> 
> In message <4B27FAF1.1070405 at freescale.com> you wrote:
> >
> > Yes, as part of the set of patches in the custodian tree.  Why introduce 
> > conflicts by targetting an older tree?  What if the new patch depends on 
> > previous patches that have gone into the custodian branch?
> 
> Custodians should send pull requests early and often to avoid big
> differences from building up.

Within reason, yes.  What happens if your "next" branch hasn't opened for a
given release yet (will you do this automatically on closing a merge
window?), or if there are some unrelated problems in your "next" branch, or
if something delays you from pulling?

Having patches float around is worse for letting big differences build up
than a few trees that for the most part cover different parts of the code. 
I don't like not having a place for a patch to go.

> > to change common practice, a general announcement would be nice rather
> > than picking an arbitrary patch to make an example out of.
> 
> I've been making this very same statement before, whenever I noticed
> such an issue. So far, it happened only very infrequently.

It may be infrequent that you notice it, but there have been a lot of
patches based on a custodian branch.

> > So I should send a request immediately after every set of patches I 
> > apply, with no time for futher testing or review?  What if the next 
> > branch hasn't opened yet?
> 
> You should never apply any patches to  your  custodian  tree  without
> review, 

I meant further review by others on the list, beyond looking at it myself.

Though, this raises the question of whether this policy would force anyone
trying to collaborate on such an experimental branch to do so off-list.

> and  normally  only  after  sufficient  testing.

I do not have the hardware to test most of the patches that I apply, nor the
time to test after each patch that gets sent.

> If you are experimenting, then use a branch for this, but never do this on
> the maste rbranch of a custodian repo.

I wasn't refering to my own experimentation, but rather giving patches
submitted by others a place to get sufficient 3rd party review and testing
before asking for a merge.

It seems like you're saying that I should wait a while before applying the
patches in the first place, requiring all testing to happen by people
manually applying the patch?  And then automatically send a pull request
every time I add patches, since at that point they're ready for merging?

That seems suboptimal for both getting wider testing of the patch before
merging to your tree, and reducing conflicts -- but if it's what you want,
fine.

The development process documentation should be updated to reflect this --
the bit you quoted comes across as a suggestion, rather than a strict
requirement, and there's this seemingly contradictory bit from
DevelopmentProcess: "In Linux, top-level maintainers will collect patches in
their trees and send pull requests to Linus as soon as the merge window
opens. So far, most U-Boot custodians do not work like that; they send pull
requests only at (or even after) the end of the merge window."

Plus, in the work flow list at the end, step 6 has the patch being added to
the custodian's git tree, and step 7 mentions a "reasonable time limit"
before requesting a pull.

-Scott