-
Notifications
You must be signed in to change notification settings - Fork 487
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
Update shared content topics #5845
Merged
Merged
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
de6b4ef
Update layout and syntax in shared topics
clayton-cornell 70bf28b
Updates to language passive voice use etc
clayton-cornell d3568dd
General updates to words syntax and voice
clayton-cornell da8b6cc
Fix sorting in table
clayton-cornell c7932d5
Fix table spacing
clayton-cornell e7a25ef
Merge branch 'main' into docs/update_shared_content
clayton-cornell 691ed16
Correct capitalization of boolean
clayton-cornell 90f1b0e
Merge branch 'main' into docs/update_shared_content
clayton-cornell fa3a76f
Apply suggestions from code review
clayton-cornell 05a5af6
Update docs/sources/shared/flow/reference/components/otelcol-debug-me…
clayton-cornell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'm not convinced that the tables should be alphabetized. Having custom ordering for the tables means we can group related arguments together, even if they don't have similar ordering, and allows us to order things in a way that increases in increasing argument specificity, with more general and common arguments towards the top of the list.
(And for a less compelling argument, people usually see forms as "username and password" but alphabetical sorting means that we're presenting it as "password and username", which is slightly jarring but not a huge deal).
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 agree with @rfratto. Also, sorting rows alphabetically will be an extra burden when writing docs :/
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.
@rfratto @ptodev I did some research on this over the past 3 days, and consulted the Grafana doc team as well. The advice is to sort lists/tables alphabetically unless the order is important to understanding the information in the list/table (for example required vs optional arguments). This guideline will be added to the Grafana Writer's Toolkit.
Semantic sorting does work (username/password is a great example), but sorting this way adds its own overhead to the doc maintenance. One person's ordering (for example sorting most-used to least used things) isn't always the next person's ordering. Semantic sorting also requires that the SME (developer/community contributor) knows where in the existing lists/tables to insert new arguments. Some people place new content at the bottom of the table/list, some in the middle. It's pretty subjective and very difficult to validate.
Other precedents for sorting alphabetically can be found in OSS documentation such as Linux man pages (which sort command arguments alphabetically) - example: https://man7.org/linux/man-pages/man1/ssh.1.html
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.
Ack, I'm fine with this outcome. Plus, this is something we could undo in the future if alpabetical sorting turns out to cause unforeseen issues.
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.
Yes, there's always a path back if it isn't working for some reason.