PING^2: resubmitted IRA improvement patches

[Kenneth Zadeck]

vlad,

i may or may not find anything interesting.    However, i will have
looked at it carefully and i am the proper reviewer.  I strongly agree
with Mark that what Richard did is not the right direction to take the
review process.   it is a very bad precedent.  Patches should be
reviewed by the proper maintainers.  Tenure should not get a patch in.

Kenny

On 10/05/2010 09:05 AM, Vladimir Makarov wrote:
>  >On 10/4/2010 9:23 AM, Vladimir Makarov wrote:
> >
> >> > I've just had a talk with Kenny.  He was kind to review the
> patches (3
> >> > of them because the 1st one is actually approved).
> >I'm glad that Kenny's reviewing the patches.  I think that, just as with
> >other patches, they should be reviewed prior to going into the compiler.
> > Please don't commit them until that review happens.
>
> Probably I could ignore your request because formally I got an
> approval from
> Richard who is a middle-end maintainer.
>
> I think Richard's proposal has more merits because I don't think Kenny
> finds a bug or major issue which can not be fixed quickly on the trunk
> (most probably it will be spelling or coding standard issues). Some
> people
> already looked at the code thoroughly, they just did not finish that
> work.
>
> But with my respect to you, I'll wait for Kenny's review first while
> doing one
> more conflict resolution from numerous ones for last 4 months.
>
> >The argument that it's a good idea to commit things early "to give them
> >as much testing as possible" doesn't really make sense to me.  Or,
> >rather, it makes sense -- but it also makes sense to review them first.
> > We know reviews tend to improve patches.  The SC hasn't been inclined
> >to grant self-review privileges to new people lately.  The only reason
> >they haven't been taken away from existing contributors (including, for
> >example, me) is because most of us aren't using those privileges widely
> >enough that it's seemed worthwhile.
>
> I think that the SC somehow monopolized a decision on reviewers and
> maintainers.  The decision should be made on a technical expertise base
> which is out of SC competence (it could be if most SC members have
> best expertise in GCC code base).
>
> But if you believe that I am wrong, I think you should revoke all
> maintainership status to be more complained to your policy (for the
> record, i'd lose insn scheduling too on which I am still active with
> producing big patches).
>
> And your argument that it is not done because you rarely use it IMHO
> works exactly opposite for me.  If you rare use it then you are losing
> expertise in your area.
>
> There are a lot maintainers which did not make a patch or even review
> a patch for years and with my point of view lost the expertise.
>
> IMHO the current most active developer(s) having most knowledge in the
> code should be a maintainer(s), other active developers should be
> reviewers.
>
> Giving maintainership status it is a question of trust in the maintainer.
> It does not mean ignoring reviews.  For example, when
> I am posting patches for insn scheduling for which I am a maintainer,
> I am asking and welcoming for comments and proposals.  When I submit
> my IRA patches, I don't ignore any review including people which has
> no reviewer or maintainer status.  The more reviews, the better for
> me.
>
> >> > Probably, I should have CCed the patches to other reviewers.
> >Yes, I think that would have helped a lot.  As a contributor, the burden
> >is on you to find reviewers.  That can be very frustrating, but it's a
> >"market-oriented" check-and-balance; it means that you have to produce
> >something compelling enough to at least one person to make it worthwhile
> >for them to review it.  My experience has been that if you present the
> >work well and ask other people nicely, it's not *that* hard to get a
> >review of something important.
> >
>
> IMHO What you wrote is just one point of view which might be does not
> work
> best for all situations.
>
> Here is my point of view.
>
> What is contributor/reviewer responsibilities is a question of half
> empty/full glass. I think it is quite wrong if a reviewer don't follow
> patches (and mailing lists).
>
> People usually don't CC to all reviewers (if any).  If a reviewer don't
> follow patches he is losing the expertise.
>
> For example, there were a lot patches (some of them are quite big)
> in IRA this year (Jeff's, Bernd's, Richard Sandiford's and others
> patches) which made IRA quite different.  All these patches
> were not CCed to all reviewers (sometimes not to one).  If other RA
> reviewers missed them, they already lost a clear picture of IRA
> with my point of view.
>
> So in my opinion, now Jeff, Bernd, and Richard Sandiford have the best
> expertise of IRA.  Also Richard Sandiford had the best expertise
> since the start.  As I remember nobody asked me who I'd recommend to
> be a IRA reviewer which I think also was wrong.
>
> I hate to be involved in political issues but sometimes it is hard to be
> silent.  Knowing that some people will start arguing with my points
> and crowd control tactics (sometimes I think it is only their single
> job),
> I am saying that this email is just expression of my thoughts and I will
> probably not waste my time in further discussion of the issues.
>
> >Certainly, with branches, the traditional practice has been to work on
> >the branch for a while and then ask for review of the branch.  Even for
> >something as large as tree-SSA, there was a review process at the end of
> >the work.
>