Page MenuHomePhabricator

Update developer notes and link to it from contributing docs
ClosedPublic

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

Diff Detail

Repository
rABC Bitcoin ABC
Branch
style-guide
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2354
Build 2843: Bitcoin ABC Buildbot (legacy)
Build 2842: arc lint + arc unit

Event Timeline

Add class doxygen comment in example.

jasonbcox requested changes to this revision.Apr 19 2018, 16:34
jasonbcox added inline comments.
doc/developer-notes.md
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
doc/developer-notes.md
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)

Braces.

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. :)

doc/developer-notes.md
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.
doc/developer-notes.md
278 ↗(On Diff #3575)

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.

359 ↗(On Diff #3575)

dito

372 ↗(On Diff #3575)

dito

doc/developer-notes.md
27 ↗(On Diff #3575)

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