Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Base branch: Safely handle divergence #4899

Merged
merged 12 commits into from
Oct 4, 2024
Merged

Base branch: Safely handle divergence #4899

merged 12 commits into from
Oct 4, 2024

Conversation

estib-vega
Copy link
Contributor

@estib-vega estib-vega commented Sep 12, 2024 •
edited
Loading

☕️ Reasoning

The application doesn't display any kind of information related to base branch divergence.
It should detect this, raise this to the user and present alternative actions to address this.

Scenario

  1. I open a repository on GitButler.
  2. A team-mate of mine makes some changes, and mistakenly pushes a commit with sensitive information.
  3. I merge the changes into the local base branch of GitButler. I can see the mistaken commit.
  4. They decide to force-push the commit away.
  5. I reload the application, but the state is the same as in step 3.

Expected behavior would be to have some information about this and the ability to restore the application to the latest version of the target upstream.

🧢 Changes

  1. Detect whether the base-branch has diverged with its upstream.
  2. Display the information to the user in a way that makes it clear if and which changes will be lost, if any action is taken.
  3. Allow the user to decide how to resolve said conflict through the Integrate Upstream Changes modal.
  4. Detect whether the base-branch has conflicts (e.g. after rebasing your local base-branch changes into the upstream) and display that.

📋 TODO

Allow the user to use edit mode on a base-branch conflict

Since we're allowing the user to rebase or merge changes into the base-branch, we can end up with conflicted commits there.
We already display and signal it, but we need to enable edit mode on base-branch conflicts.

@vercel Vercel
Copy link

vercel bot commented Sep 12, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gitbutler-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2024 4:52pm

@github-actions github-actions bot added rust Pull requests that update Rust code @gitbutler/desktop labels Sep 12, 2024
@estib-vega estib-vega force-pushed the base-branch-improvements branch 2 times, most recently from ba3329e to 901e225 Compare September 13, 2024 08:10
@estib-vega estib-vega changed the title base-branch-improvements Base branch: Safely handle divergence Sep 13, 2024
@estib-vega estib-vega marked this pull request as ready for review September 13, 2024 15:32
@krlvi
Copy link
Member

krlvi commented Sep 16, 2024

Lets separate out the update of base branch (because we can in the future show exactly what is gonna happen to branches)

estib-vega
estib-vega commented Sep 30, 2024
View reviewed changes
Copy link
Contributor Author

@estib-vega estib-vega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Byron @Caleb-T-Owens let me know what you think or if there are some other meaningful things you think I should add tests for :)

crates/gitbutler-branch-actions/src/upstream_integration.rs Outdated
Comment on lines 688 to 698

#[test]
fn test_conflicted_head_branch_resolve_divergence_hard_reset() {
let test_repository = TestingRepository::open();
let initial_commit = test_repository.commit_tree(None, &[("foo.txt", "bar")]);
let old_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "baz")]);
let branch_head = test_repository.commit_tree(Some(&old_target), &[("foo.txt", "fux")]);
// new target diverged from old target
let new_target = test_repository.commit_tree(Some(&initial_commit), &[("foo.txt", "qux")]);

let branch = make_branch(branch_head.id(), branch_head.tree_id());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Byron 👋
From here down I've added some tests for what I consider it's the main parts that should be tested:

  1. That the base-branch divergence resolution approach correctly updates the state of the branches' upstream integration statuses
  2. That the computed resolutions yield the expected results depending on the chosen base-branch divergence resolution approach

Caleb-T-Owens
Caleb-T-Owens approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@Caleb-T-Owens Caleb-T-Owens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

@estib-vega estib-vega merged commit 4de7206 into master Oct 4, 2024
18 checks passed
@estib-vega estib-vega deleted the base-branch-improvements branch October 4, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Caleb-T-Owens Caleb-T-Owens Caleb-T-Owens approved these changes

@ndom91 ndom91 Awaiting requested review from ndom91

@PavelLaptev PavelLaptev Awaiting requested review from PavelLaptev

@Byron Byron Awaiting requested review from Byron

Assignees
No one assigned
Labels
@gitbutler/desktop @gitbutler/ui rust Pull requests that update Rust code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@estib-vega @krlvi @Byron @Caleb-T-Owens