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

Update cherrypick dialog #11158

Merged
merged 11 commits into from Aug 30, 2023
Merged

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Aug 16, 2023

Proposed changes

Screenshots

Before After
image image
image image

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


✒️ I contribute this code under The Developer Certificate of Origin.

@ghost ghost assigned RussKie Aug 16, 2023
Copy link
Member

@gerhardol gerhardol 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, have not run.
Great split of commits for reviewing.
(I updated the description, adding a table to see the changes more easily.)

@RussKie
Copy link
Member Author

RussKie commented Aug 17, 2023

Looks good, have not run. Great split of commits for reviewing. (I updated the description, adding a table to see the changes more easily.)

Thank you.

I plan to merge this in the next few days, unless there are objections. I'm currently doing a lot (and I mean a lot) of rebasing and cherry-picking.

Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

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

LGTM, have not run

}

private void SaveSettings()
{
AppSettings.CommitAutomaticallyAfterCherryPick = AutoCommit.Checked;
AppSettings.AddCommitReferenceToCherryPick = checkAddReference.Checked;
if (DialogResult == DialogResult.OK)
Copy link
Member

Choose a reason for hiding this comment

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

I would not add this condition. If changed, the user has clicked the CheckBoxes.
Sometimes, I need to close a dialog in order to lookup a detail and come back...

Copy link
Member Author

Choose a reason for hiding this comment

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

Closing the dialog is essentially aborting the process. In this case, it doesn't feel right to retain the selection.

Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

The changed order of tick boxes will take some time to get used to.

@RussKie
Copy link
Member Author

RussKie commented Aug 26, 2023

The changed order of tick boxes will take some time to get used to.

I didn't realise that I had those accidentally swapped. I'll change the order. Thanks

@RussKie
Copy link
Member Author

RussKie commented Aug 29, 2023

Fixed:
image

Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

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

+0

@RussKie RussKie merged commit 002530e into gitextensions:master Aug 30, 2023
4 checks passed
@RussKie RussKie deleted the update_cherrypick_dialog branch August 30, 2023 00:58
@ghost ghost added this to the vNext milestone Aug 30, 2023
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.

None yet

3 participants