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

schancel created this revision.Apr 19 2018, 03:27
Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 19 2018, 03:27
schancel updated this revision to Diff 3550.Apr 19 2018, 14:17

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
jimpo added inline comments.Apr 19 2018, 21:14
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.

schancel updated this revision to Diff 3553.Apr 20 2018, 01:29

Update based on feedback, and include some additional suggestions

schancel updated this revision to Diff 3554.Apr 20 2018, 01:35

Delete bad examples of comments, fix a function name.

jimpo accepted this revision as: jimpo.Apr 20 2018, 03:00

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.).
schancel updated this revision to Diff 3574.Apr 23 2018, 21:03

Update block statement text

jasonbcox accepted this revision.Apr 23 2018, 22:01
This revision is now accepted and ready to land.Apr 23 2018, 22:01
This revision was automatically updated to reflect the committed changes.
deadalnix added inline comments.Apr 23 2018, 22:43
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

deadalnix added inline comments.Apr 23 2018, 22:52
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.