Re: [PATCH] Issue 1628

Branko Čibej wrote:

> Now about this style thing: When I contribute to a project, I always put
> in that extra 1% effort to get the coding style consistent. Project
> policies range from "no consitency" (e.g., Samba) through "reasonable
> consitency" (e.g., APR or HTTPD or Subversion -- for the record,
> Subversion's style is _not_ my natural style, so I have to put in that
> bit of effort when I code for SVN, too) to "Use Emacs with the GNU style
> or else", such as GCC. And yet the effort to keep the style of the
> contribution consistent is never significant, *if* one gets it through
> their head that projects /do/ have a preferred style and that they have
> a reason for that. That reason is usually to make the code more
> maintainable (I think most people will agree that having all the code in
> a consistent style _does_ make it more maintainable).

Of course, a patch should fit the style of a project before it gets
committed. But if I may suggest something:

Many people don't use the specific style Subversion uses in their
sources. And *writing* a patch in an unfamiliar style is very hard - you
can't just change your writing habits. So whenever I work on a project,
I do that with my own style. Then I send the patch in for review. Most
of the time it will have to improve in functionality/features/technical
details a few times. Then (and only then), when all those glitches are
evened out, I go over the patch again to adjust the style according to
the project.

So may I suggest that if someone sends a patch, first completely ignore
the style but just review what the patch does. The patch will get
improved, sent again and again. Then, once you think the *code* is good
enough to be committed, ask for the patch to be adjusted to the project
style. I'm sure everyone who sent the patch will be happy to do that
once it's clear that it will get committed as soon as the style is adjusted.

> In this context, the coding style is (almost) as important as giving a
> good explanation of what your patch does and why, providing a good log
> message (again, project requirements for those differ), and of course
> making the actual implementation right.

But you're talking about *maintaining* code - that's a step which
happens *after* a commit. So for a patch (as long as it's in review
state) style is irrelevant.

> Doing only part of the work and then expecting others to do the rest, as
> I've seen suggested elswhere in this thread, strikes me as being
> slightly rude. So are remarks about how one doesn't _have_ to write
> patches, implying that every patch is a royal gift that we should grovel
> over. Both tend to ignore the fact that the vast majority of the
> contributors to this project are doing it in their free time; I really
> don't see why they should support other people's lazyness.

But so do the people who write the patches - they do that in their free
time too. So by having them adjust the style while the patch is still in
review state forces them to spend even more of their free time.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org