couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Samuel Newson <rnew...@apache.org>
Subject Re: [POLL] Erlang whitespace standards
Date Mon, 31 Mar 2014 22:56:08 GMT
I went through the poll but agree with Paul that it can only be a hint. There were questions
that had multiple "right" answers but would depend on context to choose, I still picked the
"best" one in my opinion. I don’t think I’ve ever seen 8 spaces used in erlang code (well,
any code) so that option might have been wasted.

I agree we should have standards where we can, and a further question is whether we reindent
the source code to adhere to it. If we don’t, then the standard we adopt should also include
"follow local conventions when modifying existing code". if we do reindent, then we’re hurting
merge efforts right now and possibly future.

As I was doing the survey I was constantly thinking "welp, this is really a judgement call
here". I often find myself following local conventions when modifying couchdb code or cloudant
code which is not uniformly formatted (but not insanely bad either, though the further back
we go in couchdb, the worse things are, generally speaking).

To make the implicit explicit, the reason to follow local conventions is to improve the readability
of diffs for code review, to ease with subsequent merging and cherry-picking and, lastly but
obviously, a module of mixed styles is often an eye-sore worse than any single standard.

One thing I think we could quickly standardize on is whether horizontal whitespace indentation
is fixed increment or semantically aligned. I personally favor the latter, not least because
Emacs does this automatically, but I would train myself to the former if that’s the way
consensus goes. The other obvious thing we can agree on is four spaces, never tabs.

For the case question, much existing code uses the outdented style, and much new code uses
the 'proper' (i.e, erlang code outside of couchdb) style where the case clauses are indented
and the case clause expressions are indented further. I much prefer the latter, but both are
readable. I say it’s fine to write the new style for new case clauses (even if it’s a
new case clause in a function with others, however indented) and leave the old.

Does anyone feel strongly enough to push for a "reindent the codebase" epoch?

Finally, I’ll reiterate what davisp said, there were complicated expressions where none
of the indentation styles really helped, because the code itself was so gnarly. In those cases
I think we ought to refactor first (in its only actual sense, of changing only the expression
of the function, not its behavior). When we hit such cases at Cloudant we make a pull request
of multiple commits, one that does the refactoring / reformatting, and one that makes the
behavioral change or fix. These can be reviewed individually.

B.


On 31 Mar 2014, at 20:22, Paul Davis <paul.joseph.davis@gmail.com> wrote:

> There's a number of questions in there that I don't think have correct
> answers. There also wasn't a question on vertical whitespace. I can't
> see my responses but the whole thing basically boils down to breaking
> the unit/modifier theory on whitespace.
> 
> Basically, pick a unit (four spaces appears to have consensus) and
> then decide on modifiers for particular cases. For the most part the
> modifier is generally (N+1) where N is the modifier for the previous
> scope. There are a few cases where that needs to change though (ie,
> function argument continuation because it can be confusing to have it
> at the same alignment as the contents of the function).
> 
> Then there's also the question on when to avoid indentation entirely.
> For instance in the funciton invocation example (re:run/3 IIRC) I
> would've used a temporary variable for the long options parameter
> rather than try and write it directly as an argument.
> 
> Since everyone else is mentioning whitespace between functions, I much
> prefer two with a single line between function clauses when lots of
> clauses exist. The reasoning for this is that when you get a big
> function with many clauses (think complicated gen_server handle_call
> implementations) its easier to read the whole thing if there's a bit
> of whitespace, and each clause is generally independent anyway so the
> whitespace can signify that a bit. And obviously if we have one line
> of whitespace for clauses then two between functions is necessary so
> that its harder to confuse function boundaries. For functions that fit
> on a single line I tend to omit the single whitespace line and also
> for functions that are recursive bodies (ie, the standard, "foo([],
> Acc) -> lists:reverse(Acc); foo([Thing | Rest], Acc) -> foo(Rest,
> [do_thing(Thing) | Acc])." pattern.
> 
> And that's all I have to say about that for now.
> 
> 
> On Fri, Mar 28, 2014 at 4:11 PM, Joan Touzet <wohali@apache.org> wrote:
>> Time for everyone's favourite topic: whitespace standards.
>> 
>> I know many of you are fed up with not being able to auto format in
>> your favourite editor and match the CouchDB Erlang coding standards, or
>> receiving pull requests that are formatted poorly.
>> 
>> I'd like to fix that with an appropriate whitespace standard, and
>> supplementary plugins for vi and emacs that will just Do The Right Thing
>> and let us all stop worrying about whitespace corrections in pull
>> requests.
>> 
>> The basic rules seem to be:
>> 
>>    * Indent everything 4 spaces (not 2 or 8, and no tabs)
>>    * Single blank lines between functions
>>    * No blank lines between guarded versions of the
>>      same function (e.g. couch_btree.erl#L36-L47)
>> 
>> but beyond that there is some inconsistency in both the code and in
>> discussion with other devs.
>> 
>> Here's a short poll I threw together in Google Docs. I'd love it if you
>> could take 5 minutes to reply to it. Early next week I'll summarize and
>> post the results. Once we have agreement we can toss up a super small
>> Markdown guide / wiki page / whatever and get started on the emacs/vi
>> modifications to support it.
>> 
>> https://docs.google.com/forms/d/1b7KcQGgNbSCZVRwLjrUl5Z6C2TBx8X1btlU5fwrNHpg/edit#
>> 
>> Thanks,
>> Joan


Mime
View raw message