• Seb (49/49) Mar 25 2016 Hey all,
• jmh530 (2/5) Mar 25 2016 But auto functions are a key way to infer attributes.
• Timon Gehr (3/8) Mar 25 2016 Which is why it was argued by some that return type and attribute
• jmh530 (3/6) Mar 25 2016 Yes, but they are no longer orthogonal, so the above advice is
• Xinok (13/20) Mar 25 2016 I'd argue this is only applicable to some functions, not all. In
• Seb (10/20) Mar 29 2016 Yeah it would be great if we can check this automatically - see

Seb <seb wilzba.ch> writes:

Hey all,

I started to put down a list of the stuff I learned over the past 
few weeks from my involvement with Phobos. What do you think 
about reworking the contribution guide with small helpful hints 
and checks?

https://github.com/D-Programming-Language/phobos/blob/master/CONTRIBUTING.md

This list is not complete by any means - however you might also 
have small bits that you learned or words of advice that you 
would like to give to newcomers?

Code
----

- dont add  nogc,  pure,  safe attributes yourself - let the 
compiler infer it!
-> add unittest check for  nogc or  safe if possible
- use nested structs for ranges
- avoid to use `auto` as return type
- some parts of Phobos are a bit older (there are some relicts 
when the compiler was lacking functionality) - don't be afraid to 
use the new features (e.g. Foo!Range vs. Foo!(Range))
- check: did you stick to the style guide? (e.g. braces in their 
own lines (allman style) or white spaces between operators)
- check: is there an easier/more performant way to do it?
- check: is your code flexible enough to accommodate more use 
cases?
- check: did you add enough tests?
- check: read through your code again - is there any part that is 
not understandable?

Naming
------

- ping  andralex for approval of new methods/functionality

Documentation
-------------

- use backticks `code` instead of $(D foo)
- start with a capital letter in all blocks of the method header
- use $(REF) for links to other methods (format is $(REF 
_stripLeft, std, algorithm, mutation)) - $(REF_ALTTEXT) for a 
different link name
- variables will be automatically put in backticks, use an 
underscore to avoid this behavior (e.g. _foo)
- don't put examples in your ddoc header - use the `///` magic to 
annotate them.
- check: did you add Params, Returns, See_Also sections?
- check: read your documentation text aloud - is it 
understandable?
- check the output of CyberShadows's DAutoTest of the 
documentation

As a last word, a couple of this things could be enforced - 
especially the style guide. Are there already plans to check for 
this automatically?

jmh530 <john.michael.hall gmail.com> writes:

On Friday, 25 March 2016 at 07:14:52 UTC, Seb wrote:
 - dont add  nogc,  pure,  safe attributes yourself - let the 
 compiler infer it!

 - avoid to use `auto` as return type

But auto functions are a key way to infer attributes.

Timon Gehr <timon.gehr gmx.ch> writes:

On 25.03.2016 13:31, jmh530 wrote:
 On Friday, 25 March 2016 at 07:14:52 UTC, Seb wrote:
 - dont add  nogc,  pure,  safe attributes yourself - let the compiler
 infer it!

 - avoid to use `auto` as return type

 But auto functions are a key way to infer attributes.

Which is why it was argued by some that return type and attribute 
inference should be orthogonal when the feature was originally proposed.

jmh530 <john.michael.hall gmail.com> writes:

On Friday, 25 March 2016 at 14:40:34 UTC, Timon Gehr wrote:
 Which is why it was argued by some that return type and 
 attribute inference should be orthogonal when the feature was 
 originally proposed.

Yes, but they are no longer orthogonal, so the above advice is 
inconsistent.

Xinok <xinok live.com> writes:

On Friday, 25 March 2016 at 07:14:52 UTC, Seb wrote:
 - dont add  nogc,  pure,  safe attributes yourself - let the 
 compiler infer it!

I'd argue this is only applicable to some functions, not all. In 
particular, for non-templated functions,

 - avoid to use `auto` as return type

I understand the intent: "auto" is bad for documentation. 
However, auto is used a lot for type inference. So I would 
rephrase this to be clear about that, or simply add a couple 
words, "... wherever possible".

 - check: did you add enough tests?

I would extend this to emphasize "complete code coverage" in unit 
tests.

 As a last word, a couple of this things could be enforced - 
 especially the style guide. Are there already plans to check 
 for this automatically?

I believe the autotester already checks whitespace (are you using 
spaces or tabs?) though I could be wrong about this.


Overall, I think these would be good additions to the 
contribution guide.

Seb <seb wilzba.ch> writes:

On Friday, 25 March 2016 at 17:56:05 UTC, Xinok wrote:
 - check: did you add enough tests?

 I would extend this to emphasize "complete code coverage" in 
 unit tests.

Yeah it would be great if we can check this automatically - see 
the other discussion:

http://forum.dlang.org/thread/vzmkecogfcpoojauqpxw forum.dlang.org

 As a last word, a couple of this things could be enforced - 
 especially the style guide. Are there already plans to check 
 for this automatically?

 I believe the autotester already checks whitespace (are you 
 using spaces or tabs?) though I could be wrong about this.

Yep it checks for tabs and trailing whitespace, but not for the 
style guide itself (e.g. whitespace between operators or allman 
brace style)

 Overall, I think these would be good additions to the 
 contribution guide.

Thanks  all for your helpful feedback! - I opened a PR with these 
ideas to Phobos:

https://github.com/D-Programming-Language/phobos/pull/4128