Subject: Re: [PATCH 20/23] reflog_expire(): new function in the reference API

From: Jeff King <[email-masked]>
Date: Fri, 12 Dec 2014 03:50:22 -0500

On Fri, Dec 12, 2014 at 09:23:05AM +0100, Michael Haggerty wrote:

> On 12/09/2014 12:32 AM, Stefan Beller wrote:
> >> +enum expire_reflog_flags {
> >> +	EXPIRE_REFLOGS_DRY_RUN = 1 << 0,
> >> +	EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,
> >> +	EXPIRE_REFLOGS_VERBOSE = 1 << 2,
> >> +	EXPIRE_REFLOGS_REWRITE = 1 << 3
> >> +};
> > 
> > Sometimes we align the assigned numbers and sometimes we don't in git, so an alternative would be
> > 
> > enum expire_reflog_flags {
> >      EXPIRE_REFLOGS_DRY_RUN    = 1 << 0,
> >      EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,
> >      EXPIRE_REFLOGS_VERBOSE    = 1 << 2,
> >      EXPIRE_REFLOGS_REWRITE    = 1 << 3
> > }
> > 
> > Do we have a preference in the coding style on this one?

I think vertically aligned lists look really nice. But they often wreak
havoc with diffs, because introducing one longer line means re-aligning
the whole thing. IMHO, it's not worth it (but if you're going to do it,
leave lots of extra room for expansion).

Just my two cents, of course. I don't recall this particular style point
coming up before.

> Both styles are used in our codebase, and I don't think the style guide
> says anything about it. My practice in such cases is:
> 
> * If I'm modifying existing code, preserve the existing style (to avoid
> unnecessary churn)
> * If most of our code uses one style, then use that style
> * If our code uses both styles frequently, just use whatever style looks
> better to me

I think that is a very good philosophy in general.

-Peff