docs(contribution-guidelines): add best practices section for coding guidelines #112
Conversation
❌ Deploy Preview for cuhacking-portal-dev failed. Why did it fail? →
|
WalkthroughWalkthroughThe changes introduce a new section in the coding guidelines focused on best practices for end-to-end (E2E) testing. This section categorizes E2E tests by device type and emphasizes the organization and structure of tests. It includes instructions for using a Changes
Possibly related issues
Possibly related PRs
Suggested labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
apps/docs/src/content/docs/contribution-guidelines/coding-guidelines.mdx (3)
99-99: Remove redundant phrase.As suggested by the static analysis tool, use "inside" instead of "inside of" to avoid redundancy and improve clarity.
Apply this diff to fix the issue:
-All E2E tests located inside of a directory(Organized based on what overall page/tool the tests are using) +All E2E tests are located inside a directory (organized based on the overall page/tool the tests are using).Tools
LanguageTool
[style] ~99-~99: This phrase is redundant. Consider using “inside”.
Context: ...test organization All E2E tests located inside of a directory(Organized based on what ove...(OUTSIDE_OF)
100-100: Remove redundant phrase.As suggested by the static analysis tool, use "inside" instead of "inside of" to avoid redundancy and improve clarity.
Apply this diff to fix the issue:
-- For example, all E2E tests relating to the cuhacking portal are stored inside of the portal--e2e directory. +- For example, all E2E tests related to the cuhacking portal are stored inside the `portal-e2e` directory.Tools
LanguageTool
[style] ~100-~100: This phrase is redundant. Consider using “inside”.
Context: ...ting to the cuhacking portal are stored inside of the portal--e2e directory. - All rela...(OUTSIDE_OF)
140-140: Rephrase the sentence for clarity.As suggested by the static analysis tool, consider rephrasing the sentence to strengthen the wording and improve clarity.
Apply this diff to rephrase the sentence:
-- Report is produced regardless of whether you push code that makes changes to these E2E tests. +- The report is generated even if the pushed code does not modify the E2E tests.Tools
LanguageTool
[style] ~140-~140: Consider shortening or rephrasing this to strengthen your wording.
Context: ...egardless of whether you push code that makes changes to these E2E tests. When running differe...(MAKE_CHANGES)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/docs/src/content/docs/contribution-guidelines/coding-guidelines.mdx (1 hunks)
Additional context used
LanguageTool
apps/docs/src/content/docs/contribution-guidelines/coding-guidelines.mdx
[grammar] ~89-~89: Did you mean the adjective “End-to-end” (spelled with hyphens)?
Context: ...Testing* #### E2E Test catagorization End to end Testing broken down into 2 different pa...(END_TO_END_HYPHEN)
[style] ~99-~99: This phrase is redundant. Consider using “inside”.
Context: ...test organization All E2E tests located inside of a directory(Organized based on what ove...(OUTSIDE_OF)
[style] ~100-~100: This phrase is redundant. Consider using “inside”.
Context: ...ting to the cuhacking portal are stored inside of the portal--e2e directory. - All rela...(OUTSIDE_OF)
[style] ~140-~140: Consider shortening or rephrasing this to strengthen your wording.
Context: ...egardless of whether you push code that makes changes to these E2E tests. When running differe...(MAKE_CHANGES)
Additional comments not posted (6)
apps/docs/src/content/docs/contribution-guidelines/coding-guidelines.mdx (6)
88-88: Great addition!The "Best Practices" section is a valuable addition to the coding guidelines documentation. It will provide clear guidance to contributors and help maintain consistency across the codebase.
105-105: Informative note about theplaywright.configfile.The information about using a
playwright.configfile for managing tests across different environments is helpful for contributors. It provides clarity on how to handle tests for various environments.
109-109: Appropriate subsection title.The "Creating E2E tests" subsection title is clear and relevant to the content that follows. It provides a logical structure to the best practices section.
123-123: Clear naming convention guideline.The "Naming conventions" subsection title is appropriate, and the guideline for constants (all caps with words separated by underscores) is a common and widely accepted practice. It helps maintain consistency in the codebase.
126-126: Helpful subsection on running tests.The "Running tests" subsection title is appropriate, and providing information about the commands to run tests is helpful for contributors. It facilitates the testing process and ensures consistency.
137-137: Informative subsection on viewing tests.The "Viewing tests" subsection title is appropriate, and mentioning that all tests can be viewed on the GitHub workflow is helpful for contributors. It provides clarity on where to find test results and reports.
|
|
||
| ## Best Practices | ||
| ### **Testing** | ||
| #### E2E Test catagorization |
There was a problem hiding this comment.
Fix typo and use hyphenated form.
- Please fix the typo in the subsection title. It should be "categorization" instead of "catagorization".
- As suggested by the static analysis tool, use the hyphenated form "End-to-end" instead of "End to end" for consistency and clarity.
Apply this diff to fix the issues:
-#### E2E Test catagorization
-End to end Testing broken down into 2 different parts
+#### End-to-End Test Categorization
+End-to-end testing is broken down into 2 different parts: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.
| #### E2E Test catagorization | |
| #### End-to-End Test Categorization | |
| End-to-end testing is broken down into 2 different parts: |
Tools
LanguageTool
[grammar] ~89-~89: Did you mean the adjective “End-to-end” (spelled with hyphens)?
Context: ...Testing* #### E2E Test catagorization End to end Testing broken down into 2 different pa...(END_TO_END_HYPHEN)
JeremyFriesenGitHub
added
the
documentation
🚩 Linked issue
The issue is 104. The issue can be found here..
🛠 How does this PR address the issue(s)? How does it solve the problem?
The following PR is for adding a best practices section to the coding guidelines page. Best practices would describe to new contributors conventions for different areas within cuHacking's development infrastructure.
These areas are:
✅ Checklist
Make sure your PR follows the according checklist:
Code Quality
PR Review Readiness
Summary by CodeRabbit
New Features
Documentation