Add an option to adopt remote branches#616
Conversation
This is to prepare for adding remote branch adoption feature.
Current Aviator status
This PR was merged using Aviator. Stack
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
✅ FlexReview StatusCommon Owner:
Review SLO: |
Summary of ChangesHello @draftcode, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature to adopt remote branches, which will be very useful for users who work with stacked PRs created outside of av. The implementation is well-structured, leveraging the Bubble Tea framework effectively to guide the user through the process.
I've made a couple of suggestions for minor improvements, one related to performance and another to code structure for better readability.
Key highlights of the changes:
- A new
--remoteflag is added toav adopt. - A new workflow is implemented to fetch remote PR stacks, allow user selection, fetch branches, and update metadata.
- Existing actions like
AdoptTreeSelectorModelandAdoptBranchesModelhave been nicely refactored to be more generic and reusable, which is a great improvement for maintainability. - The documentation has been updated to reflect the new functionality.
Overall, this is a solid contribution. Thank you for the great work!
| var branches []actions.AdoptingBranch | ||
| for _, target := range chosenTargets { | ||
| idx := slices.IndexFunc(prs, func(prInfo actions.RemotePRInfo) bool { | ||
| return prInfo.Name == target.Short() | ||
| }) | ||
| if idx == -1 { | ||
| return uiutils.ErrCmd(fmt.Errorf("internal error: failed to find PR info for branch %s", target.Short())) | ||
| } | ||
| pr := prs[idx] | ||
| ab := actions.AdoptingBranch{ | ||
| Name: target.Short(), | ||
| Parent: pr.Parent, | ||
| PullRequest: &pr.PullRequest, | ||
| } | ||
| branches = append(branches, ab) | ||
| } |
There was a problem hiding this comment.
To improve performance, you can build a map of PRs keyed by branch name first. This avoids a linear scan (slices.IndexFunc) inside the loop, changing the complexity from O(number of PRs * number of targets) to O(number of PRs + number of targets).
prMap := make(map[string]actions.RemotePRInfo, len(prs))
for _, pr := range prs {
prMap[pr.Name] = pr
}
var branches []actions.AdoptingBranch
for _, target := range chosenTargets {
pr, ok := prMap[target.Short()]
if !ok {
return uiutils.ErrCmd(fmt.Errorf("internal error: failed to find PR info for branch %s", target.Short()))
}
ab := actions.AdoptingBranch{
Name: target.Short(),
Parent: pr.Parent,
PullRequest: &pr.PullRequest,
}
branches = append(branches, ab)
}| } | ||
|
|
||
| func (m *GetRemoteStackedPRModel) Init() tea.Cmd { | ||
| return tea.Batch(m.spinner.Tick, func() tea.Msg { |
There was a problem hiding this comment.
For better readability and testability, consider extracting this large anonymous function into a separate method on the GetRemoteStackedPRModel struct, for example, func (m *GetRemoteStackedPRModel) findPRs() tea.Msg. The Init method would then become return tea.Batch(m.spinner.Tick, m.findPRs).
aviator-app
bot
changed the base branch from
abstract_stack_renderer
to
master
2 participants
No description provided.