Skip to content

Moves Conflict state into Changes state#5901

Merged
shiftkey merged 7 commits intodid-you-merge-itfrom
put-conflict-state-in-changes-state
Oct 16, 2018
Merged

Moves Conflict state into Changes state#5901
shiftkey merged 7 commits intodid-you-merge-itfrom
put-conflict-state-in-changes-state

Conversation

@iAmWillShepherd
Copy link
Contributor

This PR addresses #5810 (comment)

@iAmWillShepherd iAmWillShepherd changed the base branch from master to did-you-merge-it October 15, 2018 20:56
Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

Looks good. I've called out some places where you can avoid merging objects when updating conflictsState to simplify the code, and one question about some refactoring.

@@ -1502,7 +1501,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
if (previousBranch.name !== currentBranchName) {
this.statsStore.recordMergeAbortedAfterConflicts()
this.repositoryStateCache.update(repository, () => ({

This comment was marked as spam.

this.repositoryStateCache.update(repository, () => ({
conflictState: {
branch: tip.branch,
changesState: {

This comment was marked as spam.


this.repositoryStateCache.update(repository, () => ({
conflictState: null,
changesState: { ...repoState.changesState, conflictState: null },

This comment was marked as spam.

const conflictsExist = repoState.conflictState !== null
const { conflictState } = repoState.changesState

if (conflictsExist) {

This comment was marked as spam.

This comment was marked as spam.

@shiftkey shiftkey added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 16, 2018
@shiftkey shiftkey added this to the 1.4.3 milestone Oct 16, 2018
@shiftkey shiftkey self-assigned this Oct 16, 2018
@shiftkey shiftkey merged commit 8308247 into did-you-merge-it Oct 16, 2018
@shiftkey shiftkey deleted the put-conflict-state-in-changes-state branch October 16, 2018 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants