-
Notifications
You must be signed in to change notification settings - Fork 13
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
Various fixes to the README #70
base: master
Are you sure you want to change the base?
Conversation
I did *not* fix the diagram to change "streaming-orcasound.net" to "audio-orcasound-net" since that file is not in this repository. That change still has to be done by someone with write access to orcasound.net/img/orcasound-app. Signed-off-by: Dave Thaler <[email protected]>
WalkthroughThe changes in the pull request primarily involve updates to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
README.md (4)
19-23
: LGTM: Improved formatting and updated information.The changes enhance readability and keep the documentation current. The update from LogDNA to Mezmo reflects the current logging service used.
Consider adding a brief note about the transition from LogDNA to Mezmo, if this change affects users or developers in any way.
41-47
: LGTM: Updated information and improved formatting.The changes keep the documentation current and enhance readability. The clearer description of NODE_NAME format is particularly helpful.
There's a minor typo in line 31: "Common to to all projects". Please remove the duplicate "to".
🧰 Tools
🪛 LanguageTool
[uncategorized] ~41-~41: When specifying a month and year, the comma is unnecessary.
Context: ...channel/437036-hydrophone-nodes). As of October, 2024, we are continuing to use AWS S3 for st...(COMMA_AFTER_A_MONTH)
[uncategorized] ~45-~45: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... * NODE_NAME should indicate your device and it's location, ideally in the form `dev...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ...rd -l`. For Raspberry Pi hardware with pisound just use AUDIO_HW_ID=pisound * CHANNELS...(AI_HYDRA_LEO_MISSING_COMMA)
79-79
: LGTM: Improved clarity of instructions.The added specificity in the instructions for copying the Docker compose file and locating the .env file is very helpful for users.
Consider adding a note about what to do if the user wants to use a different compose file, to make the instructions more comprehensive.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... For example, if you have a raspberry pi and you want to use the prebuilt image then...(COMMA_COMPOUND_SENTENCE)
[style] ~79-~79: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Watch what happens usinghtop
. If you want to verify files are being written to the `...(REP_WANT_TO_VB)
15-15
: Consider addressing minor grammatical and formatting issues.The static analysis tools have identified several minor issues that could be addressed to further improve the document:
- Line 15: Consider hyphenating "open-source" when used as a compound adjective.
- Line 31: There's a typo - "Common to to all projects" should be "Common to all projects".
- Line 41: The comma after "October" in "October, 2024" is unnecessary.
- Line 47: Consider adding a comma after "pisound" for better readability.
- Line 79: Consider adding a comma before "and" in the compound sentence for clarity.
These are minor suggestions and can be implemented at your discretion to enhance the overall quality of the documentation.
Also applies to: 31-31, 41-41, 47-47, 79-79
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...vice/OS/browser. We also aspire to use open source software as much as possible. We rely h...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~15-~15: Consider using “who” when you are referring to a person instead of an object.
Context: ...reaming_over_HTTP) segments to a player that works optimally on as many listening de...(THAT_WHO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- README.md (4 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...vice/OS/browser. We also aspire to use open source software as much as possible. We rely h...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~15-~15: Consider using “who” when you are referring to a person instead of an object.
Context: ...reaming_over_HTTP) segments to a player that works optimally on as many listening de...(THAT_WHO)
[duplication] ~31-~31: Possible typo: you repeated a word
Context: ... appropriate for your projects. Common to to all projects is the need for AWS keys ...(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~41-~41: When specifying a month and year, the comma is unnecessary.
Context: ...channel/437036-hydrophone-nodes). As of October, 2024, we are continuing to use AWS S3 for st...(COMMA_AFTER_A_MONTH)
[uncategorized] ~45-~45: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... * NODE_NAME should indicate your device and it's location, ideally in the form `dev...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ...rd -l`. For Raspberry Pi hardware with pisound just use AUDIO_HW_ID=pisound * CHANNELS...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... For example, if you have a raspberry pi and you want to use the prebuilt image then...(COMMA_COMPOUND_SENTENCE)
[style] ~79-~79: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Watch what happens usinghtop
. If you want to verify files are being written to the `...(REP_WANT_TO_VB)
🔇 Additional comments (6)
README.md (6)
5-7
: LGTM: Improved formatting and grammar.The addition of backticks around directory names enhances readability, and the grammatical correction from "There are" to "There is" improves the overall quality of the documentation.
15-15
: LGTM: Updated year range.Updating the year range to include 2024 keeps the documentation current and accurate.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...vice/OS/browser. We also aspire to use open source software as much as possible. We rely h...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~15-~15: Consider using “who” when you are referring to a person instead of an object.
Context: ...reaming_over_HTTP) segments to a player that works optimally on as many listening de...(THAT_WHO)
27-31
: LGTM: Improved technical accuracy and formatting.The correction of "X86" to "x86" enhances technical accuracy, and the consistent use of backticks for directory names improves readability.
🧰 Tools
🪛 LanguageTool
[duplication] ~31-~31: Possible typo: you repeated a word
Context: ... appropriate for your projects. Common to to all projects is the need for AWS keys ...(ENGLISH_WORD_REPEAT_RULE)
93-93
: LGTM: Added valuable resource links.The addition of links to the Google Draw source and archived schematics provides useful resources for users and contributes to the project's transparency and documentation.
97-97
: LGTM: Improved formatting consistency.The addition of backticks around the NODE_NAME variable enhances readability and maintains consistency with the formatting of other technical terms in the document.
Line range hint
1-24
: Overall, these changes significantly improve the README.The modifications in this pull request enhance the README in several ways:
- Improved formatting consistency, especially with the use of backticks for technical terms.
- Updated information to reflect the current year and services used.
- Clarified instructions for setup and deployment.
- Added valuable resource links for schematics.
These changes contribute to a more accurate, readable, and user-friendly documentation. Great work on maintaining and improving the project's documentation!
Also applies to: 27-31, 41-47, 79-79, 93-93, 97-97
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...vice/OS/browser. We also aspire to use open source software as much as possible. We rely h...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~15-~15: Consider using “who” when you are referring to a person instead of an object.
Context: ...reaming_over_HTTP) segments to a player that works optimally on as many listening de...(THAT_WHO)
[duplication] ~31-~31: Possible typo: you repeated a word
Context: ... appropriate for your projects. Common to to all projects is the need for AWS keys ...(ENGLISH_WORD_REPEAT_RULE)
🪛 Markdownlint
33-33: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Signed-off-by: Dave Thaler <[email protected]>
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
README.md (4)
19-23
: LGTM: Updated content and improved formattingThe changes improve formatting consistency and update the content:
- Added backticks around directory names for better readability.
- Updated the logging service from LogDNA to Mezmo.
- Updated the year to 2024.
These changes align with the PR objectives and improve the documentation's accuracy.
Consider adding a brief note about the transition from LogDNA to Mezmo to provide context for users familiar with the old system.
Line range hint
31-43
: LGTM: Improved instructions and updated contentThe changes enhance the documentation:
- Added backticks around directory names for consistency.
- Clarified the location for the .env file.
- Updated the communication platform from Slack to Zulip.
- Updated the year to 2024.
- Changed LogDNA references to Mezmo.
These updates improve accuracy and reflect current practices.
Consider adding a brief note about the transition from Slack to Zulip to provide context for users familiar with the old system.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... * NODE_NAME should indicate your device and it's location, ideally in the form `dev...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ...rd -l`. For Raspberry Pi hardware with pisound just use AUDIO_HW_ID=pisound * CHANNELS...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~49-~49: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...xpect (1 or 2). * FLAC_DURATION is the amount of seconds you want in each archived lo...(AMOUNTOF_TO_NUMBEROF)
[uncategorized] ~50-~50: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...ssless file. * SEGMENT_DURATION is the amount of seconds you want in each streamed lo...(AMOUNTOF_TO_NUMBEROF)
93-97
: LGTM: Added resources and improved exampleThe changes enhance the documentation:
- Added a link to archived schematics, providing more resources for users.
- Updated the example URL structure for better clarity.
These updates improve the overall quality and usefulness of the README.
Consider adding a brief description of what users can expect to find in the archived schematics to provide more context.
15-15
: Consider adding a hyphen to "open-source"For improved clarity and adherence to common style guidelines, consider changing "open source software" to "open-source software" when it's used as a compound adjective before a noun.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...vice/OS/browser. We also aspire to use open source software as much as possible. We rely h...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~15-~15: Consider using “who” when you are referring to a person instead of an object.
Context: ...reaming_over_HTTP) segments to a player that works optimally on as many listening de...(THAT_WHO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- README.md (4 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...vice/OS/browser. We also aspire to use open source software as much as possible. We rely h...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~15-~15: Consider using “who” when you are referring to a person instead of an object.
Context: ...reaming_over_HTTP) segments to a player that works optimally on as many listening de...(THAT_WHO)
[uncategorized] ~45-~45: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... * NODE_NAME should indicate your device and it's location, ideally in the form `dev...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ...rd -l`. For Raspberry Pi hardware with pisound just use AUDIO_HW_ID=pisound * CHANNELS...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... For example, if you have a raspberry pi and you want to use the prebuilt image then...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...rry pi and you want to use the prebuilt image then copydocker-compose.rpi-pull.yml
...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~79-~79: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Watch what happens usinghtop
. If you want to verify files are being written to the `...(REP_WANT_TO_VB)
🔇 Additional comments (5)
README.md (5)
5-7
: LGTM: Improved formatting and clarityThe addition of backticks around directory names (
node
,mseed
) enhances readability. The specification of "primary nodes" adds clarity to the description of the Orcasound listening network.
15-15
: LGTM: Updated yearThe year has been correctly updated from 2023 to 2024, keeping the documentation current.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...vice/OS/browser. We also aspire to use open source software as much as possible. We rely h...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~15-~15: Consider using “who” when you are referring to a person instead of an object.
Context: ...reaming_over_HTTP) segments to a player that works optimally on as many listening de...(THAT_WHO)
27-27
: LGTM: Corrected architecture capitalizationThe change from "X86" to "x86" correctly uses the standard lowercase format for the x86 architecture name, improving technical accuracy.
45-47
: LGTM: Improved readability and formattingThe changes enhance the documentation:
- Minor grammatical improvements for better readability.
- Added backticks around environment variable names (e.g.,
AUDIO_HW_ID
) for clarity.These updates improve the overall quality of the instructions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... * NODE_NAME should indicate your device and it's location, ideally in the form `dev...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ...rd -l`. For Raspberry Pi hardware with pisound just use AUDIO_HW_ID=pisound * CHANNELS...(AI_HYDRA_LEO_MISSING_COMMA)
79-79
: LGTM: Clarified Docker compose file instructionsThe instructions for copying the Docker compose file have been improved by specifying the
node
directory as the location for the .env file. This clarification enhances user understanding and aligns with the PR objective of improving the README.🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... For example, if you have a raspberry pi and you want to use the prebuilt image then...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...rry pi and you want to use the prebuilt image then copydocker-compose.rpi-pull.yml
...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~79-~79: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Watch what happens usinghtop
. If you want to verify files are being written to the `...(REP_WANT_TO_VB)
|
||
## Getting Started | ||
|
||
These instructions will get you a copy of the project up and running on your local machine for development and testing purposes. See the deployment section (below) for notes on how to deploy the project on a live system like [live.orcasound.net](https://live.orcasound.net). | ||
These instructions will get you a copy of the project up and running on your local machine for development and testing purposes. See the [deployment](#deployment) section for notes on how to deploy the project on a live system like [live.orcasound.net](https://live.orcasound.net). |
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.
Looking at Preview, perhaps making the phrase deployment section
the hypertext link, rather than just word deployment
would help the reader understand this is a README navigational link?
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 @dthaler for these great edits. Happy to approve as is, but if you want to incrementally improve here are two suggestions:
-
Line 37-38 reference LogDNA. Are these still relevant after the switch to Mezmo, @paulcretu ? If not, perhaps they should be deleted from the README (and later also on the
.env
files on the nodes)? -
Maybe we should capitalize
raspberry pi
(toRaspberry Pi
) throughout?
I did not fix the diagram to change "streaming-orcasound.net" to "audio-orcasound-net" since that file is not in this repository. That change still has to be done by someone with write access to orcasound.net/img/orcasound-app.
Among other changes, are:
Summary by CodeRabbit