-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat(core): add branch service #18379
Conversation
WalkthroughA new package named Changes
Poem
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- core/CHANGELOG.md (1 hunks)
- core/branch/branch.go (1 hunks)
Additional comments: 2
core/CHANGELOG.md (1)
- 36-39: The changelog entry for the new branch service is correctly placed under the "Unreleased" section. Ensure that the issue number and the link are correct.
core/branch/branch.go (1)
- 1-18: The interface definition looks good. It's clear and well-documented. However, ensure that the implementation of this interface correctly handles the rollback and commit operations as described in the comments. Also, ensure that the context passed to the function
f
is indeed a child of the context passed to theExecute
function.
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.
This conceptually looks right. We may want to work on the naming. I'm not sure branch
is intuitive enough.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- core/branch/branch.go (1 hunks)
Additional comments: 2
core/branch/branch.go (2)
1-19: The
branch
package andService
interface are well defined. TheExecute
method is clear in its purpose and usage. The comments provide a good explanation of the functionality. Ensure that the implementation of this interface correctly handles the context and error propagation.7-7: In response to the previous comment, the isolation refers to the execution of the function
f
in a separate context that is a child of the context passed toExecute
. Any changes made during the execution off
can be rolled back iff
returns an error, ensuring that the parent context remains unaffected. This is useful for operations that need to be atomic, where all changes should be committed only if all operations succeed.
I think we can merge, I will open an issue about this that we can mark as release blocker for core. |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- core/branch/branch.go (1 hunks)
Additional comments: 1
core/branch/branch.go (1)
- 1-19: The
Service
interface and itsExecute
method are well documented. The comments provide a clear understanding of the purpose and usage of the interface and method. The code is clean and follows Go best practices. However, ensure that the implementation of this interface correctly handles the rollback and commit scenarios as described in the comments.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
make lint
andmake test
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking changeSummary by CodeRabbit