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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit 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.
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

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

dito

372

dito

doc/developer-notes.md
27

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