Re: [PATCH v7] Add new git-related helper to contrib

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: May 31, 2013, 08:03 UTC
Message-ID: <CALkWK0nVDBv9bfFpAJ+pzON-uD_4HQ0Bcg=CN32fgFvNJnnjgA@mail.gmail.com>
In-Reply-To: <CAMP44s0peXDtFB31pEqF49xM8z5OtDKAWaOR65aZSkCndL2bhQ@mail.gmail.com>

Felipe Contreras wrote:

> I was going to make these stylistic changes to make you happy, but
> then I realized the only that does really make sense is to change msg
> = nil to msg = false, and it's not even worth to waste a thought on
> changes like that.

We don't have existing Ruby code in git.git to follow, so what I say can obviously not have more weightage than "personal opinion". Don't do things to "make me happy"; I am nobody. Have a good sense of style and defend it instead of flaming me because you thought I was stalling work.

"Some of these style changes seem to make sense, and I'll fix them after this patch gets merged."

or

"I have a different opinion on these issues, and here's why: "

would have both been appropriate responses.

> So I'll move on to the next patches, hopefully Duy or Junio would have
> some comments of actual significance, or maybe you would too, but for
> the moment it seems pretty clear you are only stating opinions about
> what Ruby code-style you like best.

I've done reviews of several iterations of this patch, and stylistic comments were all that I had left: I might not be adequately intelligent to come up with "comments of actual significance", but that's a limitation you're going to have to deal with.