C++ style

[Bob Carpenter]

[moved off issue to stan-dev]

I agree that we need to turn down the difficulty factor on
the cpplint tests at least until we agree on how picky
we're going to be. If we go full-on cpplint.py compliance, I
have a feeling all Daniel will ever do is explain errors from
it and reformat people's code, which doesn't seem productive
in a project our size. As is, he spends a substantial amount
of time correcting signed/unsigned errors which are not bugs,
but spew warnings in the more verbose warnings modes in g++.

As we can all see, people are getting frustrated with the level
of testing. We can either relax the standards, try to educate
or help people, or just tell them to lump it if they want to
contribute to Stan. I hope we don't go with the latter.

I do wish developers would read the Google style guide and my
annotations here (I trimmed it down from the previous novel to
just diffs with Google --- everything else in their style I
agree with and would be happy to comply with):

https://github.com/stan-dev/stan/wiki/Coding-Style-and-Idioms

If people have strong feelings, they should make them known.

I'm not sure what Krzysztof means by "no spaces allowed for vertical
alignment". I do not want code like this:

int x;
double y;

and so on if that's what you meant by vertical alignment.

There's also the issue of whether we want to spend a month updating
all of our code to comply. At that point, every outstanding branch
will enter merge hell. So I'm not sure how to get from where we're
at to there.

- Bob


> On Mar 28, 2015, at 8:16 AM, Daniel Lee <notifi...@github.com> wrote:
>
> Sure. I'll make a pass at the rest when I review the code.
>
> On Fri, Mar 27, 2015 at 4:49 PM, Krzysztof Sakrejda <
> notifi...@github.com> wrote:
>
> > Took care of most of the lint errors but there are some that I don't
> > understand (header order) and some that I disagree with completely (no
> > spaces allowed for vertical alignment). Reading tests is bad enough as it
> > is... anyway, if it fails, I'd appreciate some help deciding what to do
> > with the rest.
> >
> > —
> > Reply to this email directly or view it on GitHub
> > <https://github.com/stan-dev/stan/pull/1405#issuecomment-87087321>.
> >
> —
> Reply to this email directly or view it on GitHub.
>

[Krzysztof Sakrejda]

On Friday, March 27, 2015 at 7:58:30 PM UTC-4, Bob Carpenter wrote:
> [moved off issue to stan-dev]
>
> I agree that we need to turn down the difficulty factor on
> the cpplint tests at least until we agree on how picky
> we're going to be. If we go full-on cpplint.py compliance, I
> have a feeling all Daniel will ever do is explain errors from
> it and reformat people's code, which doesn't seem productive
> in a project our size. As is, he spends a substantial amount
> of time correcting signed/unsigned errors which are not bugs,
> but spew warnings in the more verbose warnings modes in g++.
>
> As we can all see, people are getting frustrated with the level
> of testing. We can either relax the standards, try to educate
> or help people, or just tell them to lump it if they want to
> contribute to Stan. I hope we don't go with the latter.
>
> I do wish developers would read the Google style guide and my
> annotations here (I trimmed it down from the previous novel to
> just diffs with Google --- everything else in their style I
> agree with and would be happy to comply with):
>
> https://github.com/stan-dev/stan/wiki/Coding-Style-and-Idioms
>
> If people have strong feelings, they should make them known.
>
> I'm not sure what Krzysztof means by "no spaces allowed for vertical
> alignment". I do not want code like this:
>
> int x;
> double y;

I don't really have strong feelings about it but I was annoyed at having cpplint complain about this:

EXPECT_FLOAT_EQ( 2.0, result(0));
EXPECT_FLOAT_EQ( 8.0, result(1));
EXPECT_FLOAT_EQ( 4.0, result(2));
EXPECT_FLOAT_EQ(10.0, result(3));
EXPECT_FLOAT_EQ( 6.0, result(4));
EXPECT_FLOAT_EQ(12.0, result(5));

I don't mind turning it into the requested:

EXPECT_FLOAT_EQ(2.0, result(0));
EXPECT_FLOAT_EQ(8.0, result(1));
EXPECT_FLOAT_EQ(4.0, result(2));
EXPECT_FLOAT_EQ(10.0, result(3));
EXPECT_FLOAT_EQ(6.0, result(4));
EXPECT_FLOAT_EQ(12.0, result(5));

But in _tests_ specifically I place some value on getting what code does at a glance. Either way I'm fine going with it if it's what you want but there are so many warnings from the rest of the code I really wasn't sure which way to go. I guess "disagree with completely" didn't mean "won't change"...

btw, (not being defensive here, just want to put the perspective out there) I did read your edits on Google's style guide but I can't keep it all in my head and I doubt most people would either without some more experience. There are a lot of moving parts to contributing to Stan "right". So if your response ends up being just "do what cpplint says" I think that works fine or if the response is "go look at the style guide again to resolve these" it works too. I don't see it as a barrier... if you went the R-help/RTFM route that would be discouraging....

Overall the experience of trying to contribute for the first time has been very positive.

Krzysztof

[Bob Carpenter]

Daniel:

What is your thinking on what we should be doing here?
I fear development is going to grind to a halt if we
try to enforce lint at this level of pickiness.

I'm for making things readable and having general style
guides, but I don't want picking lint to become a full-time
activity.

- Bob

> --
> You received this message because you are subscribed to the Google Groups "stan development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Daniel Lee]

On Fri, Mar 27, 2015 at 10:07 PM, Bob Carpenter <ca...@alias-i.com> wrote:

Daniel:

What is your thinking on what we should be doing here?
I fear development is going to grind to a halt if we
try to enforce lint at this level of pickiness.

I'm pretty sure development won't grind to a halt. I'm going to go respond to the rest of the thread and hopefully you'll agree.

 

I'm for making things readable and having general style
guides, but I don't want picking lint to become a full-time
activity.

Agreed. And it won't be full-time. I'm almost done with a pull request that will squash most of the warnings in src/stan/math. I decided it was smarter to split it across sections of the code so it'll cause less conflicts. I know how to merge it with any of the pull requests that touch math, so I can take care of that. It won't conflict with Michael's pull request or any branch that just touches the language or mcmc or services or anything else.
 

> On Mar 28, 2015, at 11:57 AM, Krzysztof Sakrejda <krzysztof...@gmail.com> wrote:
>
> On Friday, March 27, 2015 at 7:58:30 PM UTC-4, Bob Carpenter wrote:
>> [moved off issue to stan-dev]
>>
>> I agree that we need to turn down the difficulty factor on
>> the cpplint tests at least until we agree on how picky
>> we're going to be.

Agreed. Right now, there's some relaxation and it's handled inside our makefiles. If you run:

   > make cpplint

cpplint is run with our settings. If you wanted to take a look at what filters we turn on and off, see the file make/cpplint.

Here's the filter option if anyone's not wanting to dig through the code base:
  --filter=-runtime/indentation_namespace,-readability/namespace,-legal/copyright,-whitespace/indent,-runtime/reference

I think I agreed on all of Bob's comments on the styleguide. (I forget if I ever responded that way.) I think this set of options gets us close. I know we haven't decided as a group, but most of it made sense.

 

>> If we go full-on cpplint.py compliance, I
>> have a feeling all Daniel will ever do is explain errors from
>> it and reformat people's code, which doesn't seem productive
>> in a project our size.

Oh, one thing that I didn't mention: I decided not to run cpplint on our test code. (I think I talked to Rob about this decision in the office, but maybe it was an imaginary conversation.)

For one, we're not really looking for full cpplint.py compliance. We've already put in customization.

And... the error messages from cpplint are actually really instructive. It's really easy to navigate the error messages using Jenkins: just look at the new warnings. It tells you file, line number, a category, and most importantly, suggestions on how to fix it.

>>  As is, he spends a substantial amount
>> of time correcting signed/unsigned errors which are not bugs,
>> but spew warnings in the more verbose warnings modes in g++.

Yeah -- that's a bit annoying, but we started caring when RStan users got warning novels pages long due to signed/unsigned errors. I still don't know how to suppress this warning across all compiler / platform combinations, which is why I've been squashing them by hand.

 

 

>> As we can all see, people are getting frustrated with the level
>> of testing. We can either relax the standards, try to educate
>> or help people, or just tell them to lump it if they want to
>> contribute to Stan.  I hope we don't go with the latter.

I don't think that the first sentence is true in general. For those that are getting frustrated, I think we (or just I) owe it to them to write better doc to explain what's necessary.

(Personally, I'm frustrated by the lack of testing. Finding bugs after things have merged are real bummers, especially when it's clear that it could have been caught with a simple test. Writing a function and expecting it to get into Stan without at least exercising it in a couple places isn't the right expectation we should be setting for contributors. Going too far the other way isn't useful either, but I'd rather have too much than not testing it at all.)

I think the right thing is to educate and help those that want to help. When reasonable, I'm in favor of relaxing standards.

One last point: style matters. When reviewing, it's a lot easier to focus on the substance when the form of the code conforms to everything else. Everything in moderation, though.

The Jenkins job fails when the pull request introduces new warnings. So far, most of the warnings have been legit: tabs, extra spaces, etc. If we feel a few more warnings should be allowed because it makes the code much more readable, we'll do that.

>> I do wish developers would read the Google style guide and my
>> annotations here (I trimmed it down from the previous novel to
>> just diffs with Google --- everything else in their style I
>> agree with and would be happy to comply with):
>>
>> https://github.com/stan-dev/stan/wiki/Coding-Style-and-Idioms
>>
>> If people have strong feelings, they should make them known.

Thanks! I agree with just about everything you pointed out. I'm pretty sure I've written it into the list, but maybe that was just in my head. Too many emails.

 

 

>> I'm not sure what Krzysztof means by "no spaces allowed for vertical
>> alignment".  I do not want code like this:
>>
>>   int    x;
>>   double y;
>
> I don't really have strong feelings about it but I was annoyed at having cpplint complain about this:
>
> EXPECT_FLOAT_EQ( 2.0, result(0));
> EXPECT_FLOAT_EQ( 8.0, result(1));
> EXPECT_FLOAT_EQ( 4.0, result(2));
> EXPECT_FLOAT_EQ(10.0, result(3));
> EXPECT_FLOAT_EQ( 6.0, result(4));
> EXPECT_FLOAT_EQ(12.0, result(5));
>
> I don't mind turning it into the requested:
>
> EXPECT_FLOAT_EQ(2.0, result(0));
> EXPECT_FLOAT_EQ(8.0, result(1));
> EXPECT_FLOAT_EQ(4.0, result(2));
> EXPECT_FLOAT_EQ(10.0, result(3));
> EXPECT_FLOAT_EQ(6.0, result(4));
> EXPECT_FLOAT_EQ(12.0, result(5));
>
> But in _tests_ specifically I place some value on getting what code does at a glance.

So, my suggestion is to run cpplint using the make target or run it just on source. If you looked at the link I posted (http://d1m1s1b1.stat.columbia.edu:8080/job/Stan%20Pull%20Request%20-%20CppLint/93/warnings11Result/new/), it was mostly about spacing, missing spaces after commas, header include order, and length of lines.
 

>  Either way I'm fine going with it if it's what you want but there are so many warnings from the rest of the code I really wasn't sure which way to go.  I guess "disagree with completely" didn't mean "won't change"...

Ignore the warnings in test code for now. See if that helps.

 

> btw, (not being defensive here, just want to put the perspective out there) I did read your edits on Google's style guide but I can't keep it all in my head and I doubt most people would either without some more experience.  There are a lot of moving parts to contributing to Stan "right". So if your response ends up being just "do what cpplint says" I think that works fine or if the response is "go look at the style guide again to resolve these" it works too.  I don't see it as a barrier... if you went the R-help/RTFM route that would be discouraging....
>
> Overall the experience of trying to contribute for the first time has been very positive.

Thanks for sticking with us!

With your particular pull request, I ran into actual code issues, so it wasn't just the lint that was problematic. We'll figure it out. (I found it amusing that I had to look up RTFM. We obviously don't use that acronym around here.)

 

>>
>> There's also the issue of whether we want to spend a month updating
>> all of our code to comply.  At that point, every outstanding branch
>> will enter merge hell.  So I'm not sure how to get from where we're
>> at to there.

I think the plan should be:

1) don't let new pull requests add to warnings.

2) tackle sections of the code.
   a) src/stan/math: It isn't taking me that long to rip through the math. We have 13,612 warnings now. src/stan/math accounts for 9,523 of those warnings. after a few hours, I'm down to 4,105 inside src/stan/math. I don't think we'll get to 0, but we'll get low enough.

   b) src/stan/io, mcmc, model, optimization: I don't remember the last pull request we had here. Actually, there was one for io with the partial inits.

   c) src/stan/lang: wait for Bob to give the ok

   d) src/stan/command, services, interface: wait for refactor

With the warnings we can't squash reasonably, we should either relax the rule if it's a problematic rule or annotate the code to ignore the cpplint warning.

>>
>>> On Mar 28, 2015, at 8:16 AM, Daniel Lee <notifi...@github.com> wrote:
>>>
>>> Sure. I'll make a pass at the rest when I review the code.
>>>
>>> On Fri, Mar 27, 2015 at 4:49 PM, Krzysztof Sakrejda <
>>> notifi...@github.com> wrote:
>>>
>>>> Took care of most of the lint errors but there are some that I don't
>>>> understand (header order) and some that I disagree with completely (no
>>>> spaces allowed for vertical alignment). Reading tests is bad enough as it
>>>> is... anyway, if it fails, I'd appreciate some help deciding what to do
>>>> with the rest.

Krzysztof, I'll look at these when I review the pull request. I didn't see any vertical alignment warnings, but maybe you were running cpplint on tests too?

[Bob Carpenter]

See inline. Most people are going to want to ignore this until
the dust settles and Daniel gets some guidelines up on the Wiki
and the tests in Jenkins configured.

> On Mar 28, 2015, at 2:39 PM, Daniel Lee <bea...@alum.mit.edu> wrote:
>
> On Fri, Mar 27, 2015 at 10:07 PM, Bob Carpenter <ca...@alias-i.com> wrote:
> Daniel:
>
> What is your thinking on what we should be doing here?
> I fear development is going to grind to a halt if we
> try to enforce lint at this level of pickiness.
>
> I'm pretty sure development won't grind to a halt. I'm going to go respond to the rest of the thread and hopefully you'll agree.
>
>
> I'm for making things readable and having general style
> guides, but I don't want picking lint to become a full-time
> activity.
>
> Agreed. And it won't be full-time.

It's not the one-time cleaning that I'm worried about so much
as the round-trips to Jenkins plus formatting time.

But let's see. I'm willing to give it a shot.

Would you mind putting the following into a Wiki somewhere so I
don't have to fish it out of e-mail in the future?

> > On Mar 28, 2015, at 11:57 AM, Krzysztof Sakrejda <krzysztof...@gmail.com> wrote:
> >
> > On Friday, March 27, 2015 at 7:58:30 PM UTC-4, Bob Carpenter wrote:
> >> [moved off issue to stan-dev]
> >>
> >> I agree that we need to turn down the difficulty factor on
> >> the cpplint tests at least until we agree on how picky
> >> we're going to be.
>
> Agreed. Right now, there's some relaxation and it's handled inside our makefiles. If you run:
> > make cpplint
> cpplint is run with our settings. If you wanted to take a look at what filters we turn on and off, see the file make/cpplint.
>
> Here's the filter option if anyone's not wanting to dig through the code base:
> --filter=-runtime/indentation_namespace,-readability/namespace,-legal/copyright,-whitespace/indent,-runtime/reference
>
> I think I agreed on all of Bob's comments on the styleguide. (I forget if I ever responded that way.) I think this set of options gets us close. I know we haven't decided as a group, but most of it made sense.
>
>
> >> If we go full-on cpplint.py compliance, I
> >> have a feeling all Daniel will ever do is explain errors from
> >> it and reformat people's code, which doesn't seem productive
> >> in a project our size.
>
> Oh, one thing that I didn't mention: I decided not to run cpplint on our test code. (I think I talked to Rob about this decision in the office, but maybe it was an imaginary conversation.)

The issues Krzysztof brought up were in tests. So it must be at
least running over the tests.

> For one, we're not really looking for full cpplint.py compliance. We've already put in customization.

Understood. But if it won't allow the tests Krzysztof had,
that seems pretty darn picky.

> And... the error messages from cpplint are actually really instructive. It's really easy to navigate the error messages using Jenkins: just look at the new warnings. It tells you file, line number, a category, and most importantly, suggestions on how to fix it.

I haven't tried to look yet.

> >> As is, he spends a substantial amount
> >> of time correcting signed/unsigned errors which are not bugs,
> >> but spew warnings in the more verbose warnings modes in g++.
>
> Yeah -- that's a bit annoying, but we started caring when RStan users got warning novels pages long due to signed/unsigned errors. I still don't know how to suppress this warning across all compiler / platform combinations, which is why I've been squashing them by hand.

I understand the motivation. It's taken a lot of time to
get rid of them and I'm just extrapolating from that one
issue to everything cpplint.py is going to pick up. That's what
has me worried this is going to be a burden.

> >> As we can all see, people are getting frustrated with the level
> >> of testing. We can either relax the standards, try to educate
> >> or help people, or just tell them to lump it if they want to
> >> contribute to Stan. I hope we don't go with the latter.
>
> I don't think that the first sentence is true in general.

I won't speak for Krzysztof, but I'm getting frustrated just
seeing all the mail on these low-level formatting issues.

> For those that are getting frustrated, I think we (or just I) owe it to them to write better doc to explain what's necessary.

Well, there's the Google guide, and I guess it did
say not to include spaces inside function calls.

Adding the explicit exceptions and providing some instructions
on how to run cpplint locally (though maybe it's automatic now,
I haven't tried) and how to look at things on Jenkins would help.
But it's just one more hurdle to getting started. These things add
up. It's less onerous for those of us who already have many of
them internalized (well, I still mess up GitHub on a regular basis,
so maybe not internalized).

> (Personally, I'm frustrated by the lack of testing. Finding bugs after things have merged are real bummers, especially when it's clear that it could have been caught with a simple test. Writing a function and expecting it to get into Stan without at least exercising it in a couple places isn't the right expectation we should be setting for contributors. Going too far the other way isn't useful either, but I'd rather have too much than not testing it at all.)

This is a much bigger issue in my mind than formatting.
Agreed that we absolutely need to insist on tests before merging.

> I think the right thing is to educate and help those that want to help. When reasonable, I'm in favor of relaxing standards.

I think the issue here is that reasonable people can disagree!

> One last point: style matters. When reviewing, it's a lot easier to focus on the substance when the form of the code conforms to everything else. Everything in moderation, though.

Agreed. I don't want whacky styles like we see from users for
their R or Stan code. Or some of the Boost styles! They clearly
let each package do whatever they want.

> The Jenkins job fails when the pull request introduces new warnings. So far, most of the warnings have been legit: tabs, extra spaces, etc. If we feel a few more warnings should be allowed because it makes the code much more readable, we'll do that.

I'll have to trust you have it under control. The last thing I submitted
had a gazillion errors and none of them were tabs.

Extra space is where people tend to get creative and I'm happy with
that in moderation or in cases where it's justified for readability.
But then we have to go through one by one and judge each case because
it won't get through Jenkins otherwise.

> >> I do wish developers would read the Google style guide and my
> >> annotations here (I trimmed it down from the previous novel to
> >> just diffs with Google --- everything else in their style I
> >> agree with and would be happy to comply with):
> >>
> >> https://github.com/stan-dev/stan/wiki/Coding-Style-and-Idioms
> >>
> >> If people have strong feelings, they should make them known.
>
> Thanks! I agree with just about everything you pointed out. I'm pretty sure I've written it into the list, but maybe that was just in my head. Too many emails.

Too many emails is another issue! I'm the main contributor on
that front :-(

So we're agreed that we'll insist on the second form here and
not allow the first? Or can users do that cpplint override thing
if they want? And if so, how do we review that?

> >> I'm not sure what Krzysztof means by "no spaces allowed for vertical
> >> alignment". I do not want code like this:
> >>
> >> int x;
> >> double y;
> >
> > I don't really have strong feelings about it but I was annoyed at having cpplint complain about this:
> >
> > EXPECT_FLOAT_EQ( 2.0, result(0));
> > EXPECT_FLOAT_EQ( 8.0, result(1));
> > EXPECT_FLOAT_EQ( 4.0, result(2));
> > EXPECT_FLOAT_EQ(10.0, result(3));
> > EXPECT_FLOAT_EQ( 6.0, result(4));
> > EXPECT_FLOAT_EQ(12.0, result(5));
> >
> > I don't mind turning it into the requested:
> >
> > EXPECT_FLOAT_EQ(2.0, result(0));
> > EXPECT_FLOAT_EQ(8.0, result(1));
> > EXPECT_FLOAT_EQ(4.0, result(2));
> > EXPECT_FLOAT_EQ(10.0, result(3));
> > EXPECT_FLOAT_EQ(6.0, result(4));
> > EXPECT_FLOAT_EQ(12.0, result(5));
> >
> > But in _tests_ specifically I place some value on getting what code does at a glance.
>
> So, my suggestion is to run cpplint using the make target or run it just on source. If you looked at the link I posted (http://d1m1s1b1.stat.columbia.edu:8080/job/Stan%20Pull%20Request%20-%20CppLint/93/warnings11Result/new/), it was mostly about spacing, missing spaces after commas, header include order, and length of lines.

OK --- is that what Jenkins is doing? I'm confused why Krzysztof
brought up the test issue.

>
> > Either way I'm fine going with it if it's what you want but there are so many warnings from the rest of the code I really wasn't sure which way to go. I guess "disagree with completely" didn't mean "won't change"...
>
> Ignore the warnings in test code for now. See if that helps.
>
>
> > btw, (not being defensive here, just want to put the perspective out there) I did read your edits on Google's style guide but I can't keep it all in my head and I doubt most people would either without some more experience. There are a lot of moving parts to contributing to Stan "right". So if your response ends up being just "do what cpplint says" I think that works fine or if the response is "go look at the style guide again to resolve these" it works too. I don't see it as a barrier... if you went the R-help/RTFM route that would be discouraging....
> >
> > Overall the experience of trying to contribute for the first time has been very positive.
>
> Thanks for sticking with us!
>
> With your particular pull request, I ran into actual code issues, so it wasn't just the lint that was problematic. We'll figure it out. (I found it amusing that I had to look up RTFM. We obviously don't use that acronym around here.)

I do (use the acronymy), but nobody does (read the manual).
The problem is that the manual's too long and it quickly goes
stale. The wiki needs an updating pass.

> >>
> >> There's also the issue of whether we want to spend a month updating
> >> all of our code to comply. At that point, every outstanding branch
> >> will enter merge hell. So I'm not sure how to get from where we're
> >> at to there.
>
> I think the plan should be:
> 1) don't let new pull requests add to warnings.
> 2) tackle sections of the code.
> a) src/stan/math: It isn't taking me that long to rip through the math. We have 13,612 warnings now. src/stan/math accounts for 9,523 of those warnings. after a few hours, I'm down to 4,105 inside src/stan/math. I don't think we'll get to 0, but we'll get low enough.
> b) src/stan/io, mcmc, model, optimization: I don't remember the last pull request we had here. Actually, there was one for io with the partial inits.
> c) src/stan/lang: wait for Bob to give the ok
> d) src/stan/command, services, interface: wait for refactor

We'll have to see how (1) goes.

I'm OK on src/stan/lang, but it's going to be a doozy and
I really think you're going to see that it needs lots of
exceptions because of the crazy template metaprogramming that's
completely unreadable under the normal formatting rules. I'm
not trying to get crazy here --- you'll see what I mean when you
try to squash.

> With the warnings we can't squash reasonably, we should either relax the rule if it's a problematic rule or annotate the code to ignore the cpplint warning.

I do not want to get in the habit of cluttering our code with
comments to suppress lint. I'd rather just turn the tests off.

- Bob

[Krzysztof Sakrejda]

I don't want to add to the novella, so I snipped most of it, but:

On Saturday, March 28, 2015 at 3:41:31 AM UTC-4, Bob Carpenter wrote:
[snip]

> > Oh, one thing that I didn't mention: I decided not to run cpplint on our test code. (I think I talked to Rob about this decision in the office, but maybe it was an imaginary conversation.)
>
> The issues Krzysztof brought up were in tests. So it must be at
> least running over the tests.

I got sick of the Jenkins round trip and ran cpplint (with your options) directly on the new files I was contributing (the make target wouldn't work for me locally for some reason). I didn't realize Jenkins didn't run it on tests. Oops.

[snip]

> > With your particular pull request, I ran into actual code issues, so it wasn't just the lint that was problematic. We'll figure it out. (I found it amusing that I had to look up RTFM. We obviously don't use that acronym around here.)
>
> I do (use the acronymy), but nobody does (read the manual).
> The problem is that the manual's too long and it quickly goes
> stale. The wiki needs an updating pass.

Maybe the Wiki needs more info but I disagree with the gist of this. The problem is that there's too much to digest at the start of contributing. For most people it's going to involve multiple attempts no matter what the wiki/manual say.

That's where cpplint is great since it keeps you guys from having to explain most of the formatting complaints. It might help to have some Stan-specific doc only where lint is ambiguous like what header order should be.

[Bob Carpenter]

> On Mar 28, 2015, at 11:27 PM, Krzysztof Sakrejda <krzysztof...@gmail.com> wrote:
>
> I don't want to add to the novella,

So much for finishing my novel :-)

...

>> The problem is that the manual's too long and it quickly goes
>> stale. The wiki needs an updating pass.
>
> Maybe the Wiki needs more info but I disagree with the gist of this.

I take it the disagreement is about adding more doc, not where it goes.

> The problem is that there's too much to digest at the start of contributing. For most people it's going to involve multiple attempts no matter what the wiki/manual say.

Point taken.

> That's where cpplint is great since it keeps you guys from having to explain most of the formatting complaints. It might help to have some Stan-specific doc only where lint is ambiguous like what header order should be.

OK. Thanks for the feedback. Sounds like Daniel was right
and we haven't yet made it too onerous. Which is good news.

- Bob

[Krzysztof Sakrejda]

On Friday, March 27, 2015 at 11:39:50 PM UTC-4, Daniel Lee wrote:
> Krzysztof, I'll look at these when I review the pull request. I didn't see any vertical alignment warnings, but maybe you were running cpplint on tests too?

Thanks, I did have myself turned around and I ran your lint command on my tests as well. The make target didn't work directly because it expects python2 and just silently exits with python3. Also I didn't have cpplint in my path so I just downloaded it locally and hacked the make target to make it go. I'll figure out something more sensible.

Krzysztof

[Daniel Lee]

Bob, I reread the Google Style Guide and all your comments. I agree with you on all of it.

I updated the wiki page:
https://github.com/stan-dev/stan/wiki/Coding-Style-and-Idioms

I've updated the order to match the Google Style Guide, added links to the appropriate sections, and marked the exceptions that lead to cpplint rule changes.

I'm creating a pull request to include a distribution of the Google C++ style guide and cpplint so we'll have a common version. I didn't have to update any of the rules.

Daniel