Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Replace coding.txt with Bitcoin developer-notes.md #2186

Draft
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

jamescowens
Copy link
Member

This PR is a tracking/discussion PR that starts with replacing our severely outdated coding.txt style guide with Bitcoin's developer notes markdown document. The Bitcoin style is what the core devs have been following in large part with the new code for Gridcoin.

I am marking this as draft, and welcome discussion on this to fine tune it for our blockchain with additional change commits prior to merging.

@jamescowens jamescowens added this to the Janice milestone Jun 20, 2021
@jamescowens jamescowens self-assigned this Jun 20, 2021
@jamescowens
Copy link
Member Author

In particular I will start out with referencing a discussion the core devs were having in #2177. @cyrossignol made some good comments which I will repeat below:

As an aside, these are the rough guidelines for managing legacy code style that seem standard in most teams that I've worked on:

  • Code in new modules should always follow the style guide
  • When modifying legacy code, be consistent with the style of the surrounding scope
    • Typically, a function boundary, but it could be a small file/class or a large nested block
    • Prefer to avoid mixing cosmetic changes with domain changes (batch-commit formatting as a whole separately)
    • New scopes can follow the style guide to ease expected transitions later
  • Do not modify the style of upstream code
  • Rely on tools for style enforcement rather than manual review
    • Use linters to flag style violations
    • Use formatters to apply style before submission
    • "Policy, not personal"

Of course, we need an up-to-date style guide for any of this to really matter. I think we do a decent job following these points so far (I surely haven't done so absolutely myself). I mention these because formalizing our position should save us time—I'd rather discuss and review style changes as little as possible. No convention or tool set is perfect, though. The goal is to minimize the distraction of code style from the review process—both for new reviews (PRs) and historical (git log/blame, change tracking, etc.)—so I propose these guidelines as a starting point.

Note that bullet number 2 above is different than the stance taken in Bitcoin developer-notes.md in the Coding Style (General) section.

doc/developer-notes.md Outdated Show resolved Hide resolved
doc/developer-notes.md Outdated Show resolved Hide resolved
showing up in the command list.

- Use *invalid* bech32 addresses (e.g. in the constant array `EXAMPLE_ADDRESS`) for
`RPCExamples` help documentation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not applicable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think it is. We should not be using valid Gridcoin address in RPC examples.

doc/developer-notes.md Outdated Show resolved Hide resolved
doc/developer-notes.md Outdated Show resolved Hide resolved
@jamescowens jamescowens modified the milestones: Janice, Kermit's Mom Nov 26, 2021
@jamescowens jamescowens modified the milestones: Kermit's Mom, LaVerne Jul 17, 2022
@jamescowens jamescowens modified the milestones: LaVerne, Miss Piggy Feb 2, 2023
jamescowens and others added 2 commits March 23, 2023 14:22
This replaces our severely outdated coding.txt style guide with Bitcoin's
developer notes markdown document. The Bitcoin style is what the core devs
have been following in large part with the new code for Gridcoin.
@jamescowens jamescowens modified the milestones: Miss Piggy, Natasha Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants