docs: Extend CONTRIBUTING.md with code guidelines#4339
Conversation
- Add Code Guidelines section with naming conventions, type conventions, JSON tags, URL tags, pagination, and common types - Move Code Comments under Code Guidelines - Remove duplicate content from Submitting a patch, Metadata, Scripts sections - Fix gofmt formatting for all code snippets - Incorporate reviewer feedback from google#4202 (alexandear, stevehipwell) - Address gmlewis feedback on keeping CONTRIBUTING.md concise
CLA Fix SummaryClosed PR #4299 (and #4202) due to CLA check failures caused by:
What I did to fix:
Result: PR #4339 now has 1 commit with clean CLA verification ✅ The CONTRIBUTING.md content is identical to what was in #4299 — all reviewer feedback incorporated (alexandear, stevehipwell, gmlewis). Ready for review! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4339 +/- ##
=======================================
Coverage 97.50% 97.50%
=======================================
Files 193 193
Lines 19451 19451
=======================================
Hits 18965 18965
Misses 269 269
Partials 217 217 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
There was a problem hiding this comment.
One nit, otherwise LGTM.
| ... | ||
| } | ||
| ``` | ||
| And the returned type `Repository` will have comments like this: | ||
|
|
||
| ```go | ||
| // Repository represents a GitHub repository. | ||
| type Repository struct { | ||
| ID *int64 `json:"id,omitempty"` | ||
| ID *int64 `json:"id,omitempty"` | ||
| NodeID *string `json:"node_id,omitempty"` | ||
| Owner *User `json:"owner,omitempty"` | ||
| // ... | ||
| Owner *User `json:"owner,omitempty"` | ||
| ... |
There was a problem hiding this comment.
In my opinion, these changes to the code sections are a regression and the new formatting looks worse than the original. I think that the changes to the new lines 174-185 should be reverted.
Revert the formatting changes to the code examples in the Code Comments section to match the original style with tabs and // ... comments, as requested in review feedback from gmlewis.
|
I've addressed the nit from the review - reverted the code formatting changes in the Code Comments examples to match the original style with tabs and |
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @jlaportebot!
LGTM.
stevehipwell
left a comment
There was a problem hiding this comment.
LGTM
I do still think that using the Repository struct in the example for a response object is unclear as it's two most significant fields (ID & Name) are pointers when they are required and by the coding standards logic should not be.
This is an orthogonal issue. I believe it was originally done this way because, depending upon the endpoint where either Feel free to file a separate issue so that someone can investigate if this is no longer the case in any of the DotCom or current or legacy Enterprise versions of GitHub so that they can be changed. |
@gmlewis I was very careful not to say that the |
Extends CONTRIBUTING.md with comprehensive code guidelines for contributors, including:
Addresses feedback from maintainers on the initial submission.