Merged
Conversation
📝 WalkthroughWalkthroughAdds Appendix A on submodules to CONTRIBUTING.md and updates the PR template to reference it; Appendix A documents testing conventions (two tests per command including Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Line 123: Edit the sentence to correct punctuation and formatting: add a comma
after "features", wrap both paths in backticks (`src/api/` and `src/commands/`)
for consistency, and ensure the `discoverGitdir` and `gitdir` identifiers are
formatted in backticks where mentioned (e.g., "apply the `discoverGitdir`
function" and "send the `gitdir` value through the `discoverGitdir` filter") so
all technical symbols are consistently styled.
- Line 121: Remove the trailing period from the subsection header text "3.
Creating new `src/api/` commands." so it matches the other headers; update the
header to "3. Creating new `src/api/` commands" (no period) wherever that exact
header appears to maintain consistent formatting across subsections.
- Line 119: Update the documentation text in CONTRIBUTING.md by replacing the
lowercase acronym "api" with the capitalized "API" in the sentence that reads
'Let's imagine "brancher" is a new api command.'; locate the same phrase and
change "api" to "API" so the line reads '... a new API command' to maintain
consistent technical capitalization.
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
127-127: Improve phrasing for clarity.The phrase "perhaps nothing must change" is awkward. Consider rephrasing for better readability.
📝 Suggested improvement
-Depending on the situation, perhaps nothing must change. See the notes above regarding the `discoverGitdir` function. +Depending on the situation, no changes may be necessary. See the notes above regarding the `discoverGitdir` function.
CONTRIBUTING.md
Outdated
|
|
||
| 2. Creating new `__tests__` for brand new apis | ||
|
|
||
| Let's imagine "brancher" is a new api command. Place two new files in `__tests__` which are `test-brancher.js` and `test-brancher-in-submodule.js`. They should be mostly identical. For submodule tests, import and use `makeFixtureAsSubmodule` instead of `makeFixture`. See the notes above about the `gitdir` variable. |
There was a problem hiding this comment.
Capitalize "API" for consistency.
Technical acronyms should be capitalized. Change "api" to "API" to align with standard technical writing conventions.
📝 Suggested fix
-Let's imagine "brancher" is a new api command. Place two new files in `__tests__` which are `test-brancher.js` and `test-brancher-in-submodule.js`. They should be mostly identical. For submodule tests, import and use `makeFixtureAsSubmodule` instead of `makeFixture`. See the notes above about the `gitdir` variable.
+Let's imagine "brancher" is a new API command. Place two new files in `__tests__` which are `test-brancher.js` and `test-brancher-in-submodule.js`. They should be mostly identical. For submodule tests, import and use `makeFixtureAsSubmodule` instead of `makeFixture`. See the notes above about the `gitdir` variable.📝 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.
Suggested change
| Let's imagine "brancher" is a new api command. Place two new files in `__tests__` which are `test-brancher.js` and `test-brancher-in-submodule.js`. They should be mostly identical. For submodule tests, import and use `makeFixtureAsSubmodule` instead of `makeFixture`. See the notes above about the `gitdir` variable. | |
| Let's imagine "brancher" is a new API command. Place two new files in `__tests__` which are `test-brancher.js` and `test-brancher-in-submodule.js`. They should be mostly identical. For submodule tests, import and use `makeFixtureAsSubmodule` instead of `makeFixture`. See the notes above about the `gitdir` variable. |
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` at line 119, Update the documentation text in
CONTRIBUTING.md by replacing the lowercase acronym "api" with the capitalized
"API" in the sentence that reads 'Let's imagine "brancher" is a new api
command.'; locate the same phrase and change "api" to "API" so the line reads
'... a new API command' to maintain consistent technical capitalization.
CONTRIBUTING.md
Outdated
|
|
||
| Let's imagine "brancher" is a new api command. Place two new files in `__tests__` which are `test-brancher.js` and `test-brancher-in-submodule.js`. They should be mostly identical. For submodule tests, import and use `makeFixtureAsSubmodule` instead of `makeFixture`. See the notes above about the `gitdir` variable. | ||
|
|
||
| 3. Creating new `src/api/` commands. |
There was a problem hiding this comment.
Remove period for consistency.
Subsection headers 1, 2, and 4 don't have periods, but subsection 3 does. Remove the period to maintain consistent formatting.
📝 Suggested fix
-3. Creating new `src/api/` commands.
+3. Creating new `src/api/` commands📝 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.
Suggested change
| 3. Creating new `src/api/` commands. | |
| 3. Creating new `src/api/` commands |
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` at line 121, Remove the trailing period from the subsection
header text "3. Creating new `src/api/` commands." so it matches the other
headers; update the header to "3. Creating new `src/api/` commands" (no period)
wherever that exact header appears to maintain consistent formatting across
subsections.
CONTRIBUTING.md
Outdated
|
|
||
| 3. Creating new `src/api/` commands. | ||
|
|
||
| In terms of submodule-related features only modify src/api/ files and not src/commands/. This is an architectural decision to keep logic at one layer of the stack, while other layers may remain unaffected. Review other files, the basic idea is to apply the `discoverGitdir` function, and never assume `gitdir` is right. Send the `gitdir` value through the `discoverGitdir` filter before passing it anywhere else. In a common situation, when submodules aren't used, the `discoverGitdir` filter will just send back the original value. If it turns out a submodule is used it will return the required information. |
There was a problem hiding this comment.
Fix grammar and formatting.
This line has multiple minor issues:
- Missing comma after "features"
- Inconsistent use of backticks around file paths (
src/api/andsrc/commands/should both have backticks)
📝 Suggested fix
-In terms of submodule-related features only modify src/api/ files and not src/commands/. This is an architectural decision to keep logic at one layer of the stack, while other layers may remain unaffected. Review other files, the basic idea is to apply the `discoverGitdir` function, and never assume `gitdir` is right. Send the `gitdir` value through the `discoverGitdir` filter before passing it anywhere else. In a common situation, when submodules aren't used, the `discoverGitdir` filter will just send back the original value. If it turns out a submodule is used it will return the required information.
+In terms of submodule-related features, only modify `src/api/` files and not `src/commands/`. This is an architectural decision to keep logic at one layer of the stack, while other layers may remain unaffected. Review other files, the basic idea is to apply the `discoverGitdir` function, and never assume `gitdir` is right. Send the `gitdir` value through the `discoverGitdir` filter before passing it anywhere else. In a common situation, when submodules aren't used, the `discoverGitdir` filter will just send back the original value. If it turns out a submodule is used it will return the required information.📝 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.
Suggested change
| In terms of submodule-related features only modify src/api/ files and not src/commands/. This is an architectural decision to keep logic at one layer of the stack, while other layers may remain unaffected. Review other files, the basic idea is to apply the `discoverGitdir` function, and never assume `gitdir` is right. Send the `gitdir` value through the `discoverGitdir` filter before passing it anywhere else. In a common situation, when submodules aren't used, the `discoverGitdir` filter will just send back the original value. If it turns out a submodule is used it will return the required information. | |
| In terms of submodule-related features, only modify `src/api/` files and not `src/commands/`. This is an architectural decision to keep logic at one layer of the stack, while other layers may remain unaffected. Review other files, the basic idea is to apply the `discoverGitdir` function, and never assume `gitdir` is right. Send the `gitdir` value through the `discoverGitdir` filter before passing it anywhere else. In a common situation, when submodules aren't used, the `discoverGitdir` filter will just send back the original value. If it turns out a submodule is used it will return the required information. |
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` at line 123, Edit the sentence to correct punctuation and
formatting: add a comma after "features", wrap both paths in backticks
(`src/api/` and `src/commands/`) for consistency, and ensure the
`discoverGitdir` and `gitdir` identifiers are formatted in backticks where
mentioned (e.g., "apply the `discoverGitdir` function" and "send the `gitdir`
value through the `discoverGitdir` filter") so all technical symbols are
consistently styled.
sdarwin
force-pushed
the
feature/doc
branch
3 times, most recently
from
6a857de to
b00caab
Compare
Member
|
Maybe it's worth updating the PR template with information about it as well. |
Contributor
Author
Done. |
Member
|
🎉 This PR is included in version 1.36.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
If anyone submits a pull request which modifies
__tests__or adds new api commands, comment to them when necessary, "Please refer to Appendix A submodules in the CONTRIBUTING.md file. Make sure to include submodule tests."Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.