Re: Potentially merging the transactional-memory branch into mainline.

[Michael Matz <matz at suse dot de>]

Hi,

On Thu, 3 Nov 2011, Aldy Hernandez wrote:

> Have you not seen the last 3 years of patches to gcc-patches?  They're 
> prefixed by [trans-mem].  Perhaps you're filtering them out.

That's not how things are supposed to be, sorry.  The private branches 
don't require reviews, and hence also won't get (as much) review.  Patches 
proposed for trunk get review.

> > For instance, there are new files gcc/c-pretty-print.c or 
> > ipa-type-escape.c, gcc/c.opt is moved meanwhile to gcc/c-family.  All 
> > these files aren't mentioned in the ChangeLog.  At this point I simply 
> > stopped looking carefully at the diff myself, because clearly nobody 
> > else did.  No wonder it's 800k.  I don't think this is in a reviewable 
> > state
> 
> That was a minor rsync hiccup this morning that clobbered the previous 
> patch that was on the website.  The aforementioned files got picked up 
> incorrectly by the svn merge.  Hit reload on your browser,

Yes, it's better now.  Sorry for assuming this was the patch that was 
supposed to be applied and jumping to conclusions.  That also would have 
been solved by just following processes we have.

> or better yet... download the branch.

Downloading a branch also isn't required for review.  What is required to 
get patches into GCC is documented on our web page, and it's the duty of 
the proposer to follow that.  Amongst them is sending to gcc-patches@ the 
final proposed patch, split in managable pieces.  I think we can allow 
some leeway for this splitting (i.e. not be too anal with not mixing 
different changes) when the patch is for a branch merge, but the sending 
to the mailing list has very good reasons, namely to make it easy for 
reviewers to send comments.  Developing a private branch for three years 
in the open is no substitute for that.

> > right now. It might simply be a misapplied merge, or a broken diff, 
> > but whatever it is, I'd expect at least reviewable patches before 
> > considering a merge.
> 
> Reviewable patches as in what goes in gcc-patches?  Or do you want 
> something else?

Reviewable patches to gcc-patches at the point when you (or anybody else) 
propose to apply them to trunk.  Specifically _not_ during development on 
the branch, exactly because nobody except the developers of it review much 
of that.  You see, reviews are also supposed to catch things that 
developers involved with the branch simply don't see anymore, or things 
that weren't done "properly" on the branch because one wanted to move 
forward with other issues and then forgotten about, or simply the usual 
style nits.


Ciao,
Michael.