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.

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

Support for resolving conflicts when pulling from remote git branch. [INS-4550] #8118

Merged
merged 7 commits into from
Nov 5, 2024

Conversation

yaoweiprc
Copy link
Contributor

@yaoweiprc yaoweiprc commented Oct 23, 2024

Changes:

  • Users can now resolve conflicts in app when pulling from remote branch.
  • You must commit your local changes first to avoid losing data.
  • For each conflict file, you can choose either your version or their version.
  • A new commit will be created after resolving conflicts.
image

@filfreire filfreire force-pushed the feat/resolve-conflict-git-pull branch from 4787645 to cb79001 Compare October 23, 2024 10:59
@yaoweiprc yaoweiprc force-pushed the feat/resolve-conflict-git-pull branch from cb79001 to 4ebda98 Compare October 24, 2024 06:12
@filfreire filfreire force-pushed the feat/resolve-conflict-git-pull branch from 4ebda98 to 259161b Compare October 25, 2024 09:23
@filfreire
Copy link
Member

rebased with latest develop and fixed lint issue

@filfreire filfreire requested review from jackkav, gatzjames, CurryYangxx and a team October 25, 2024 09:25
@yaoweiprc yaoweiprc force-pushed the feat/resolve-conflict-git-pull branch 2 times, most recently from f9783da to 9339772 Compare October 29, 2024 06:48
@CurryYangxx
Copy link
Member

Could you add some description to let us know what changes were made?

@yaoweiprc yaoweiprc force-pushed the feat/resolve-conflict-git-pull branch from a41d721 to 4e49ff7 Compare October 31, 2024 07:08
gatzjames
gatzjames previously approved these changes Oct 31, 2024
@gatzjames
Copy link
Contributor

Did some testing and it works great! LGTM!

Next steps for Git:

  • Use the same mechanism for merge conflicts when merging branches
  • Show alert when users switch branches and have changes that will be lost
  • Expand e2e tests to cover these cases

console.log('[git] Pull remote=origin', await this.getCurrentBranch());
return git.pull({
...this._baseOpts,
...gitCallbacks(gitCredentials),
remote: 'origin',
singleBranch: true,
}).catch(
async err => {
Copy link
Member

Choose a reason for hiding this comment

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

I suggest that the logic in catch can be split into several separate functions. The current function is too long and difficult to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

commitMessage: string;
commitParent: string[];
}) {
console.log('[git] continue to merge after resolving merge conflicts', await this.getCurrentBranch());
Copy link
Member

Choose a reason for hiding this comment

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

Does this log need to be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do need this log.

@yaoweiprc yaoweiprc merged commit c385971 into develop Nov 5, 2024
8 checks passed
@yaoweiprc yaoweiprc deleted the feat/resolve-conflict-git-pull branch November 5, 2024 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants