-
Notifications
You must be signed in to change notification settings - Fork 52
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 strawman contributing guidelines #672
base: main
Are you sure you want to change the base?
Conversation
|
||
If you are willing to follow the conditions of this license, check out the following links | ||
to find an unassigned item to contribute to: | ||
|
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 find the fact that there's multiple places to look below pretty confusing as a potential contributor. How do I decide which one to look at? Are they redundant? Is one more important than the others? Do I need to look at all of them when picking?
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.
Good point! The first 3 ultimately link back to the issues page. I don't think we're directly picking issues from Figma until requested by the design team, in which case we make an issue here. I don't think we're actively using Trello these days, so we can probably remove those.
the bar for both quality and design/roadmap appropriateness will be merged into the source. | ||
|
||
For all pull requests the following rules apply: | ||
- The pull request description should describe the problem and solution, and reference the GitHub issue if one exists. |
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 can find existing PRs like #635 that don't follow this rule.
However, I still recommend that this rule should apply.
The language about the license comes from what github shows at the top of https://github.com/orcasound/orcasite/blob/main/LICENSE. I don't like the use of this license, and Scott's discussion in orcasound#181 proposes changing it, which I would prefer. However, in this PR I just document the current state, which may dissuade some contributors from participating. Changing the current state, if done, would be in a separate pull request. Signed-off-by: Dave Thaler <[email protected]>
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe Changes
Assessment against linked issues
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 (7)
CONTRIBUTING.md (7)
Line range hint
1-11
: LGTM! Consider adding anchors to improve navigation.The introduction and table of contents look great. The restructuring provides a clear overview of the document. To further improve navigation, consider adding anchor links to each section in the table of contents.
Here's an example of how to add anchor links:
- [Code of Conduct](#code-of-conduct) - [Reporting Bugs](#reporting-bugs) - [Requesting New Features](#requesting-new-features) - [Contributing Code](#contributing-code) - [Contacting Us](#contacting-us) - [Resources](#resources)🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: Possible missing article found.
Context: ...casound/orcasite/issues). If you found bug, you can help us by [submitting a GitHu...(AI_HYDRA_LEO_MISSING_THE)
[style] ~70-~70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~73-~73: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~83-~83: A determiner appears to be missing. Consider inserting it.
Context: ...ery feature that is end-user visible. - Coding style should be consistent with the sty...(AI_EN_LECTOR_MISSING_DETERMINER)
[uncategorized] ~97-~97: Possible missing article found.
Context: ...ns) ### Meetings We meet for one hour other Wednesday via Google Meet. You can req...(AI_HYDRA_LEO_MISSING_THE)
🪛 Markdownlint
6-6: null
Link fragments should be valid(MD051, link-fragments)
7-7: null
Link fragments should be valid(MD051, link-fragments)
16-24
: LGTM! Consider minor improvements for clarity and completeness.The "Reporting Bugs" section provides clear instructions for contributors. Here are a few suggestions to enhance it further:
On line 21, add the article "a" before "bug" for grammatical correctness:
-If you found bug, you can help us by +If you found a bug, you can help us byConsider adding a note about including relevant system information and logs when reporting bugs. This can help developers reproduce and fix issues more efficiently.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: Possible missing article found.
Context: ...casound/orcasite/issues). If you found bug, you can help us by [submitting a GitHu...(AI_HYDRA_LEO_MISSING_THE)
38-41
: Consider clarifying the implications of the AGPL license.While the license information is clearly stated, it might be helpful to briefly explain the implications of the AGPL license for potential contributors. This could include mentioning that any modifications to the code must also be released under the AGPL, which may affect how the code can be used in other projects.
45-51
: LGTM! Consider prioritizing the issue selection resources.The list of resources for finding issues to work on is comprehensive. However, it might be helpful to prioritize or categorize these resources to guide contributors more effectively. For example:
- GitHub issues (Good first issues, Help wanted)
- Project management tools (Project board, Trello board, Public roadmap)
- Design resources (Figma)
This organization could help contributors focus on the most relevant resources based on their interests and skills.
73-74
: Minor: Add a hyphen to "Open-Source" for clarity.To improve readability and adhere to common style guidelines for compound adjectives, consider adding a hyphen:
-[Open Source Contribution Etiquette](http://tirania.org/blog/archive/2010/Dec-31.html) by Miguel de Icaza and +[Open-Source Contribution Etiquette](http://tirania.org/blog/archive/2010/Dec-31.html) by Miguel de Icaza and🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....(EN_COMPOUND_ADJECTIVE_INTERNAL)
85-111
: LGTM! Consider a minor grammar adjustment.The "Contacting Us" section provides comprehensive information on how to get in touch with the project team and participate in meetings. To improve clarity, consider this minor adjustment:
-We meet for one hour other Wednesday via Google Meet. +We meet for one hour every other Wednesday via Google Meet.This change makes the meeting schedule clearer to potential participants.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~97-~97: Possible missing article found.
Context: ...ns) ### Meetings We meet for one hour other Wednesday via Google Meet. You can req...(AI_HYDRA_LEO_MISSING_THE)
Line range hint
112-150
: LGTM! Consider expanding the example codebases section.The "Resources" section is comprehensive and provides valuable learning materials for contributors. The inclusion of both official documentation and community resources is excellent.
To further improve this section, consider expanding the example codebases part:
- Add a brief description of why each codebase is relevant or what aspects of it are particularly useful for Orcasite contributors.
- If possible, include more examples that demonstrate different aspects of the project (e.g., frontend, backend, testing strategies).
This additional context could help contributors better understand how these examples relate to the Orcasite project.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: Possible missing article found.
Context: ...casound/orcasite/issues). If you found bug, you can help us by [submitting a GitHu...(AI_HYDRA_LEO_MISSING_THE)
[style] ~70-~70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~73-~73: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~83-~83: A determiner appears to be missing. Consider inserting it.
Context: ...ery feature that is end-user visible. - Coding style should be consistent with the sty...(AI_EN_LECTOR_MISSING_DETERMINER)
[uncategorized] ~97-~97: Possible missing article found.
Context: ...ns) ### Meetings We meet for one hour other Wednesday via Google Meet. You can req...(AI_HYDRA_LEO_MISSING_THE)
🪛 Markdownlint
6-6: null
Link fragments should be valid(MD051, link-fragments)
7-7: null
Link fragments should be valid(MD051, link-fragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- CONTRIBUTING.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~21-~21: Possible missing article found.
Context: ...casound/orcasite/issues). If you found bug, you can help us by [submitting a GitHu...(AI_HYDRA_LEO_MISSING_THE)
[style] ~70-~70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~73-~73: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~83-~83: A determiner appears to be missing. Consider inserting it.
Context: ...ery feature that is end-user visible. - Coding style should be consistent with the sty...(AI_EN_LECTOR_MISSING_DETERMINER)
[uncategorized] ~97-~97: Possible missing article found.
Context: ...ns) ### Meetings We meet for one hour other Wednesday via Google Meet. You can req...(AI_HYDRA_LEO_MISSING_THE)
🪛 Markdownlint
CONTRIBUTING.md
6-6: null
Link fragments should be valid(MD051, link-fragments)
7-7: null
Link fragments should be valid(MD051, link-fragments)
🔇 Additional comments (3)
CONTRIBUTING.md (3)
12-15
: LGTM! Clear reference to the Code of Conduct.This section effectively directs contributors to the external Code of Conduct file. It's concise and emphasizes the importance of understanding the Code of Conduct before contributing.
26-34
: LGTM! Clear guidelines for feature requests and proposals.This section effectively outlines the process for requesting new features and proposing implementations. It emphasizes the importance of communication and early feedback, which is crucial for successful contributions.
Line range hint
1-150
: Great job on improving the contribution guidelines!The restructuring and additions to the CONTRIBUTING.md file significantly enhance its clarity, completeness, and usefulness for potential contributors. The new sections on reporting bugs, requesting features, and available resources are particularly valuable. The guidelines now provide a comprehensive roadmap for contributors, from initial contact to submitting pull requests.
While there are a few minor suggestions for improvement, the overall quality of the document is excellent. These guidelines will undoubtedly help streamline the contribution process and make it more accessible to newcomers.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: Possible missing article found.
Context: ...casound/orcasite/issues). If you found bug, you can help us by [submitting a GitHu...(AI_HYDRA_LEO_MISSING_THE)
[style] ~70-~70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~73-~73: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~83-~83: A determiner appears to be missing. Consider inserting it.
Context: ...ery feature that is end-user visible. - Coding style should be consistent with the sty...(AI_EN_LECTOR_MISSING_DETERMINER)
[uncategorized] ~97-~97: Possible missing article found.
Context: ...ns) ### Meetings We meet for one hour other Wednesday via Google Meet. You can req...(AI_HYDRA_LEO_MISSING_THE)
🪛 Markdownlint
6-6: null
Link fragments should be valid(MD051, link-fragments)
7-7: null
Link fragments should be valid(MD051, link-fragments)
|
||
If you are willing to follow the conditions of this license, check out the following links | ||
to find an unassigned item to contribute to: | ||
|
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.
Good point! The first 3 ultimately link back to the issues page. I don't think we're directly picking issues from Figma until requested by the design team, in which case we make an issue here. I don't think we're actively using Trello these days, so we can probably remove those.
Co-authored-by: skanderm <[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: 3
🧹 Outside diff range and nitpick comments (1)
CONTRIBUTING.md (1)
72-72
: Fix grammatical issue in reference text.Apply this diff to fix the compound adjective:
- two blogs posts + two blog posts🧰 Tools
🪛 LanguageTool
[uncategorized] ~72-~72: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....(EN_COMPOUND_ADJECTIVE_INTERNAL)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
CONTRIBUTING.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[grammar] ~21-~21: There is an agreement error between ‘found’ and ‘bug’. Insert ‘a(n)’ or change the noun to plural.
Context: ...com/orcasound/orcasite/issues). If you found bug, you can help us by [submitting a GitHu...
(PRP_VB_NN)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~72-~72: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
CONTRIBUTING.md
6-6: null
Link fragments should be valid
(MD051, link-fragments)
7-7: null
Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (2)
CONTRIBUTING.md (2)
77-82
: LGTM! Clear and comprehensive PR guidelines.
These PR requirements are well-defined and align with best practices. While some existing PRs may not follow all these guidelines, it's important to enforce them going forward to maintain code quality and documentation standards.
Line range hint 1-111
: LGTM! Well-structured contribution guidelines.
The document successfully achieves its goal of documenting the current state of contribution guidelines. The structure is clear and comprehensive, making it easy for potential contributors to understand the process and requirements.
🧰 Tools
🪛 LanguageTool
[grammar] ~21-~21: There is an agreement error between ‘found’ and ‘bug’. Insert ‘a(n)’ or change the noun to plural.
Context: ...com/orcasound/orcasite/issues). If you found bug, you can help us by [submitting a GitHu...
(PRP_VB_NN)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~72-~72: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
6-6: null
Link fragments should be valid
(MD051, link-fragments)
7-7: null
Link fragments should be valid
(MD051, link-fragments)
Signed-off-by: Dave Thaler <[email protected]>
…to contributing
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: 1
🧹 Outside diff range and nitpick comments (2)
CONTRIBUTING.md (2)
38-41
: Improve license information readability.The license information paragraph could be better formatted for clarity.
Consider this formatting:
-Orcasite is currently covered by the [GNU Affero General Public License v3.0](https://github.com/orcasound/orcasite/blob/main/LICENSE). Permissions of this strongest copyleft license are conditioned on making available -complete source code of licensed works and modifications, which include larger works using a licensed -work, under the same license. Copyright and license notices must be preserved. Contributors provide -an express grant of patent rights. +Orcasite is currently covered by the [GNU Affero General Public License v3.0](https://github.com/orcasound/orcasite/blob/main/LICENSE). + +Key license conditions: +- Complete source code of licensed works and modifications must be made available +- Larger works using the licensed work must use the same license +- Copyright and license notices must be preserved +- Contributors provide an express grant of patent rights
74-79
: Enhance PR requirements visibility.The PR requirements could be formatted more clearly for better visibility.
Consider this formatting:
-For all [pull requests](https://github.com/orcasound/orcasite/pulls) the following rules apply: -- The pull request description should describe the problem and solution, and reference the GitHub issue if one exists. -- Existing tests should continue to pass. -- Tests should ideally be included for every bug or feature that is completed. -- Documentation should be included for every feature that is end-user visible. -- Coding style should be consistent with the style used in other files of the same type in this repository. +### Pull Request Requirements + +All [pull requests](https://github.com/orcasound/orcasite/pulls) must meet the following criteria: + +1. **Description**: Clearly describe the problem and solution, referencing any related GitHub issues +2. **Tests**: + - All existing tests must pass + - New tests should be included for bugs and features +3. **Documentation**: Include documentation for user-visible features +4. **Code Style**: Follow the repository's existing coding style
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
CONTRIBUTING.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[style] ~66-~66: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~69-~69: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (5)
CONTRIBUTING.md (5)
5-10
: LGTM: Well-structured table of contents.
The table of contents is clear, comprehensive, and all section links are properly formatted.
12-14
: LGTM: Clear reference to Code of Conduct.
The section appropriately directs contributors to read the Code of Conduct before contributing.
16-24
: LGTM: Comprehensive bug reporting guidelines.
The section provides clear steps for reporting bugs, including checking for duplicates and guidelines for detailed reports.
26-34
: LGTM: Clear feature request process.
The section effectively outlines the process for requesting and implementing new features, emphasizing community feedback.
Line range hint 111-156
: LGTM: Excellent learning resources section.
The resources section is well-organized and provides comprehensive learning materials for both Elixir and React/JS development.
🧰 Tools
🪛 LanguageTool
[style] ~66-~66: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...x) feel free to start working on a fix. If you are submitting a feature or substan...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~69-~69: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... two blogs posts on contributing code: [Open Source Contribution Etiquette](http://tirania....
(EN_COMPOUND_ADJECTIVE_INTERNAL)
- [Trello board with overview of features](https://trello.com/b/hRFh7Sc1/orcasite-development) | ||
- [Public roadmap](https://trello.com/b/wBg0qhss/orcasound-roadmap) |
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.
🛠️ Refactor suggestion
Remove outdated Trello board links.
Based on previous discussions, Trello is no longer actively used. These links should be removed to avoid confusion.
-- [Trello board with overview of features](https://trello.com/b/hRFh7Sc1/orcasite-development)
-- [Public roadmap](https://trello.com/b/wBg0qhss/orcasound-roadmap)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- [Trello board with overview of features](https://trello.com/b/hRFh7Sc1/orcasite-development) | |
- [Public roadmap](https://trello.com/b/wBg0qhss/orcasound-roadmap) |
The language about the license comes from what github shows at the top of https://github.com/orcasound/orcasite/blob/main/LICENSE. I don't like the use of this license, and Scott's discussion in #181 proposes changing it, which I would prefer. However, in this PR I just document the current state, which may dissuade some contributors from participating. Changing the current state, if done, would be in a separate pull request.
Fixes #671
Summary by CodeRabbit
CONTRIBUTING.md
file.