-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 style guide and basic contribution section #3651
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's only in draft but left some comments to call out a couple ideas.
Thank you for taking this on!
Thank you for engaging in this conversation! Also, to have this here, copying @chalin's comment (open-telemetry/community#1828 (comment)):
|
So, the structure works for me, although I am not 100% sure about the content of each page:
|
I strongly second what @svrnm has shared (in the previous comment #3651 (comment)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, you can probably just replace the mermaid
shortcodes by plain fenced code blocks:
```mermaid
...
```
@chalin Thanks! I'm open to moving the guide, etc. to |
@svrnm Applied most changes and fixed linter issues. This should now serve as a base for further changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the time and effort to write this down @theletterf , here are some suggestions
Co-authored-by: Severin Neumann <neumanns@cisco.com>
Co-authored-by: Severin Neumann <neumanns@cisco.com>
Co-authored-by: Severin Neumann <neumanns@cisco.com>
Thanks @chalin! I hope we're not letting through anything egregious, copy wise — we've undergone several review rounds already. Fully agree on iterating afterwards. Re: contribution-guidelines.md, I've chosen to keep it there, as it's not visible in the ToC anyway. I've made some edits to add a link to the new contribution section. What's the purpose of this hidden doc, if I may ask? On gerunds in headings and such, this PR already aims to apply Google Style Guide's guidance. It's slightly different for file names and directories though, so I've renamed the main folder. Note: The concurrent mode for link checking is working well! |
That page was created way-back because Docsy requires that file to exist if we use it's default community page content. IIRC, we didn't want to have to fuss with the file, given that #897 had been filed, and so we chose to simply not show it in the left nav. My preference would be that you remove that file (and ensure that the content is covered elsewhere), though we can do that later if y'all prefer. There are a few of my originally suggested changes that remain to be done:
Yes, I'm so glad it works so well 😄 |
@chalin Thanks! Comments addressed. Let's remove that file in a later update. |
@svrnm There seem to be an error with links in Edit: Updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chalin Thanks! Comments addressed.
Thanks for following through with those suggested updates.
Let's remove that file in a later update.
Ok, in that case:
- Let's keep Contribution guidelines needs rework or should be dropped #897 open -- i.e., pls remove the "fix" mention in the opening comment
- Drop the change to
.htmltest.yml
since that assumed you'd be deleting/docs/contribution-guidelines
You'll need to rebase and push (I can't from the web UI):
<!-- prettier-ignore-start --> | ||
| Term | Usage | | ||
| --- | --- | | ||
| OpenTelemetry | OpenTelemetry should always be capitalized. Don't use Open-Telemetry. | | ||
| OTel | OTel is the accepted short form of OpenTelemetry. Don't use OTEL. | | ||
| Collector | When referring to the OpenTelemetry Collector, always capitalize Collector. | | ||
| Repository | Code repository, lowercase when in the middle of a sentence. Don't use "repo" or "repos". | | ||
| OTEP | OpenTelemetry Enhancement Proposal. Write "OTEPs" as plural form. Don't write "OTep" or "otep". | | ||
| OpAMP | Open Agent Management Protocol. Don't write "OPAMP" or "opamp" in descriptions or instructions. | | ||
| OTLP | OpenTelemetry Protocol. Don't write "OTlP" or "otlp" in descriptions or instructions. | | ||
<!-- prettier-ignore-end --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment for later: I don't think that we should duplicate here in the form of a table any checks that the tools do. IMHO, we should just mention the tools and how to invoke them so that contributors know if they're using the proper spelling and capitalization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a strong educational value in having our terminology presented as reference. This is not too different from what style guides do with their wordlists:
- https://developers.google.com/style/word-list
- https://docs.splunk.com/Documentation/StyleGuide/current/StyleGuide/Usagedictionary
- https://learn.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/m/microsoft-store
Having a place that explains, for humans, what the proper capitalization or spelling is for OTel terminology also has the potential of ending discussions and preventing headaches during editing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the links. I agree that it can make sense for both the terms OpenTelemetry and OTel, but not e.g. for Repository or simple cases of all caps for acronyms. But we can discuss these finer points later.
Also, if we can automate the creation of such a table somehow, that would satisfy my need to keep things DRY.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree on the generic terminology! Automation would be great, too.
In general, while deeply imperfect, I see this initial PR as the foundation for deeper discussions around our contribution guidelines and style. We'll get to a great point soon, I'm sure!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #3846
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stellar! 🚀
@svrnm Seems like we can go live! :) |
Initial structure and a good chunk of the style guide are taken from the Kubernetes docs site, with adaptations.
Preview: