-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
perf(FormResetAnotherBranch): Use quicker command #12058
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
Conversation
Enable OK button using dry-run of previous command
Set BackColor in order to indicate what is wrong or that the check is going on
@pmiossec update-ref was discussed when the command was added. Do you remember why it was not used? |
|
I have not found such discussion and from what I remembered it was from the beginning using 'push' because I remembered that it was a clever use of it to have this security check. But I might be wrong because I didn't remembered that I contributed this very useful feature 😅 I still have to find some time* to have a look at the PR but it seems to be a clever move as the security check is still there. If the dry-run deliver the perf improvement, it's great. I wanted to have a look at it to be sure we don't have a regression at the security check because it is a very important point for me. But I was really waiting for the perf improvement feedback from the user and as far as I known we still not have it... *: and it will maybe be difficult in the near future due to some health problems taking some time and energy to settle... |
Skip check if ForceReset.Checked Add cancellation
Do not enable Ok button if branch is invalid
This was done so long ago that I have no clue. But it's very likely that I didn't know of the |
ComboBox.TextChanged and ComboBox.BackColor are not fun!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The case where the branch to reset is automatically filled in the combobox (FormResetAnotherBranch.cs
line 78) is not well handled.
Normally it's the case where you sync the branch with its remote and the button should nearly every times be enabled by default.
The Validate()
event is well called after the branch name is set but the 1st pre-requisit in the Validate() method is not satisfied and so the button is not enabled.
Validate default candidate at once Validate once
Fixes #12035
Proposed changes
OK
button using a dry-run of the previous command "push"Screenshots
N/A
Test methodology
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.