Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upFix: git tab keeps popping open when there's a merge conflict #1822
+253
−17
Conversation
vanessayuenn
added some commits
Dec 4, 2018
vanessayuenn
changed the title
Fix: git tab keeps popping open when there's a merge conflict
Dec 4, 2018
This comment has been minimized.
This comment has been minimized.
coveralls
commented
Dec 4, 2018
•
vanessayuenn
added some commits
Dec 5, 2018
vanessayuenn
changed the title
[wip] Fix: git tab keeps popping open when there's a merge conflict
Dec 5, 2018
annthurium
added this to In Progress 🔧
in Stability Sprint : 20 November 2018 - 8 January 2019 : v0.24.0
Dec 5, 2018
Stability Sprint : 20 November 2018 - 8 January 2019 : v0.24.0
automation
moved this from In Progress 🔧
to QA Review 🔬
Dec 6, 2018
| @@ -72,6 +72,7 @@ export default function({confirm, notificationManager, workspace}) { | |||
| return result; | |||
| } catch (error) { | |||
| if (error instanceof GitError) { | |||
| repository.didPullError(); | |||
This comment has been minimized.
This comment has been minimized.
smashwilson
Dec 6, 2018
Member
What's the reason for generalizing this from specifically merge errors to any pull errors?
| import {GitError} from '../lib/git-shell-out-strategy'; | ||
|
|
||
|
|
||
| describe('getRepoPipelineManager()', function() { |
This comment has been minimized.
This comment has been minimized.
| @@ -453,7 +452,7 @@ export default class RootController extends React.Component { | |||
| componentDidUpdate() { | |||
| this.subscription.dispose(); | |||
| this.subscription = new CompositeDisposable( | |||
| this.props.repository.onMergeError(() => this.gitTabTracker.ensureVisible()), | |||
| this.props.repository.onPullError(() => this.gitTabTracker.ensureVisible()), | |||
This comment has been minimized.
This comment has been minimized.
smashwilson
referenced this pull request
Dec 6, 2018
Merged
Disable the "changes" status from CodeCov #1827
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
@smashwilson when I was testing it, I have found the merge error check we have does not capture all cases where merge conflicts arise (e.g. in my case it's because I have set my |
vanessayuenn
merged commit b1f5c03
into
master
Dec 11, 2018
Stability Sprint : 20 November 2018 - 8 January 2019 : v0.24.0
automation
moved this from QA Review 🔬
to Merged ☑️
Dec 11, 2018
vanessayuenn
deleted the
vy/merge-conflict-pane
branch
Dec 11, 2018
This comment has been minimized.
This comment has been minimized.
|
Ohh autorebase trips it up, huh. Good call |
This was referenced Jan 2, 2019
Closed
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
vanessayuenn commentedDec 4, 2018
•
edited
Description of the Change
This PR fixes the bug where the git tab keeps opening itself when a merge conflict is detected and remains unfixed. Instead of doing that, I moved the error signaling up one level, so now all git errors resulted from a
pullaction will trigger an error pop up, and git tab will open itself (one time only).get-repo-pipeline-manager.jsAlternate Designs
N/A
Benefits
Users won't get annoyed at git tab opening itself anymore. People don't like it when robots act smarter than them, y'know.
Applicable Issues
Resolves #1816
Metrics
N/A
Tests
See above