Page MenuHomePhabricator

Update developer notes and link to it from contributing docs

Authored by schancel on Apr 19 2018, 03:27.

Diff Detail

rABC Bitcoin ABC
Lint Not Applicable
Tests Not Applicable

Event Timeline

Add class doxygen comment in example.

jasonbcox requested changes to this revision.Apr 19 2018, 16:34
jasonbcox added inline comments.
28 ↗(On Diff #3550)

Curly braces should be on the same line as the definition. Same goes for lines below this one.

53 ↗(On Diff #3550)

Rather than what this code does, perhaps "Comment summarizing the intended purpose of this section of code". Unless the code is particularly difficult to parse, describing what it does is a bit redundant. The intended purpose is often very similar to what it does, but we all know that isn't always the case. :P

324 ↗(On Diff #3550)

Should we leave this in until T214 is completed?

This revision now requires changes to proceed.Apr 19 2018, 16:34
18 ↗(On Diff #3550)

Remove the allowance for single-line if statements.

39 ↗(On Diff #3550)

Presumably lowerCamelCase for local variables too? Can we document that somewhere?

44 ↗(On Diff #3550)

Can you make it explicit that this applies to both functions and class methods?

56 ↗(On Diff #3550)


Update based on feedback, and include some additional suggestions

Delete bad examples of comments, fix a function name.

One last clarification on block statements and I think it's good. :)

18 ↗(On Diff #3554)

These are called "block statements". We can clarify this by saying:

  • Always add braces for block statements (if, for, while, etc.).

Update block statement text

This revision is now accepted and ready to land.Apr 23 2018, 22:01
This revision was automatically updated to reflect the committed changes.

Why was this removed ? This is good advice and should be adapted. Plus it has nothing to do with style guide so has nothing to do in that diff anyway.






This isn't a formating rule. While this is a good rule, it has nothing to do burried in the middle of formatting rules.