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

Add coding style guide #2831

Merged
merged 2 commits into from
Nov 9, 2020
Merged

Add coding style guide #2831

merged 2 commits into from
Nov 9, 2020

Conversation

mway
Copy link
Collaborator

@mway mway commented Nov 3, 2020

This PR adds the M3 style guide, which is a superset of Uber's Go Style Guide (that is, unless explicitly specified/overridden, M3 will follow Uber's style guide by default).

@mway mway added the TSC Requires attention from the TSC label Nov 3, 2020
@ChrisChinchilla
Copy link
Collaborator

I have a writing style guide in progress too, so one tiny piece of feedback would be to update this to refer to a coding style guide.

@mway
Copy link
Collaborator Author

mway commented Nov 5, 2020

I have a writing style guide in progress too, so one tiny piece of feedback would be to update this to refer to a coding style guide.

This is a coding style guide - it's an umbrella guide to add on top of the Uber style guide, which we're adopting here as well. Can you give some examples of what types of things you'd like to add guidance around? Happy to work with you to incorporate things into the Uber guide, or to add to this one.

@ChrisChinchilla
Copy link
Collaborator

I have a writing style guide in progress too, so one tiny piece of feedback would be to update this to refer to a coding style guide.

This is a coding style guide - it's an umbrella guide to add on top of the Uber style guide, which we're adopting here as well. Can you give some examples of what types of things you'd like to add guidance around? Happy to work with you to incorporate things into the Uber guide, or to add to this one.

I get that, but I'm asking that it's clearer in this doc that it's a coding style guide, at the moment it just says "style guide", basically just update the heading, add that in the opening paragraphs etc. That's about it, minor stuff I know, but just so when I add the written word style guide it's more obvious where people should look. It may be that when I add the written word style guide, some further tweaks will be needed, but I can handle that then :)

@mway
Copy link
Collaborator Author

mway commented Nov 5, 2020

I get that, but I'm asking that it's clearer in this doc that it's a coding style guide, at the moment it just says "style guide", basically just update the heading, add that in the opening paragraphs etc. That's about it, minor stuff I know, but just so when I add the written word style guide it's more obvious where people should look. It may be that when I add the written word style guide, some further tweaks will be needed, but I can handle that then :)

Ah! Understood, my bad. I'll update this to be clearer. Thanks!

@mway mway changed the title Add style guide Add coding style guide Nov 5, 2020
@ChrisChinchilla
Copy link
Collaborator

Ah! Understood, my bad. I'll update this to be clearer. Thanks!

Amazing, thanks!

Copy link
Collaborator

@robskillington robskillington left a comment

Choose a reason for hiding this comment

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

LGTM, great!

@robskillington robskillington merged commit 9ba35ad into master Nov 9, 2020
@robskillington robskillington deleted the mway/style branch November 9, 2020 21:20
soundvibe added a commit that referenced this pull request Nov 11, 2020
* master: (28 commits)
  [dbnode] Add claims for index segments volume index (#2846)
  [dbnode] Remove namespaces from example config and integration tests (#2866)
  [dbnode] Resurrect flaky test skip (#2868)
  [aggregator] Fix checkCampaignStateLoop (#2867)
  [dbnode] implement deletion method in namespace kvadmin service (#2861)
  Replace closer with resource package (#2864)
  Add coding style guide (#2831)
  Add GOVERNANCE.md to describe governance (#2830)
  Add COMPATIBILITY.md to describe version compatibility (#2829)
  Refactor etcd config as discovery section with convenience types (#2843)
  Refactor x/lockfile into dbnode/server (#2862)
  [lint] Disable nlreturn linter (#2865)
  [m3cluster] Expose placement algorithm in placement service (#2858)
  [etcd] Set reasonable cluster connection/sync settings by default (#2860)
  [dbnode] Use bits.LeadingZeros64 to improve encoder performance (#2857)
  Cleanup m3nsch leftovers (#2856)
  Update ci-scripts to correct coverage tracking (#2854)
  [aggregator] Process campaign state without waiting for first campaign check interval (#2855)
  Bump go to 1.14 (#2853)
  [query] Remove single series error from M3
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TSC Requires attention from the TSC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants