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.

Sign up for GitHub

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

Jump to bottom

DeleteUnusedBranches plugin: Fix branch deletion #10663

Merged
merged 1 commit into from Jan 23, 2023
Merged

DeleteUnusedBranches plugin: Fix branch deletion #10663

merged 1 commit into from Jan 23, 2023

Conversation

pmiossec
Copy link
Member

@pmiossec pmiossec commented Jan 20, 2023 •
edited

Remove a duplicated git delete command run that will always return an exit code different than 0 (because the 1st command already deleted the branch)

because the plugin doesn't handle well exception thrown when exit code is not zero (new development done in v4.0 where exception is thrown by default)

Fixes #10662

Screenshots

Test methodology

  • Manually

Test environment(s)

  • Git Extensions 33.33.33
  • Build ca3100f (Dirty)
  • Git 2.38.1.windows.1
  • Microsoft Windows NT 10.0.19045.0
  • .NET 6.0.13
  • DPI 96dpi (no scaling)

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.

@pmiossec
DeleteUnusedBranches plugin: Fix branch deletion
eff9339 
Remove a duplicated git command run that will always return an exit code different than 0
(because the 1st command already deleted it)

because the plugin doesn't handle well exception thrown when exit code is not zero
(new development done in v4.0 where exception is thrown by default)

Fixes gitextensions#10662
@ghost ghost assigned pmiossec Jan 20, 2023
@pmiossec pmiossec mentioned this pull request Jan 20, 2023
Closed
@pmiossec
Copy link
Member Author

@RussKie @maraf @mstv @gerhardol

FYI, the retrieve of the "plugin manager" in Download-PluginManager.ps1 during the build exceed the GitHub api rate limits and so break all the last builds (for example: https://ci.appveyor.com/project/gitextensions/gitextensions/builds/45996325 )

@RussKie
Copy link
Member

RussKie commented Jan 20, 2023 via email •
edited by ghost

@RussKie
Copy link
Member

RussKie commented Jan 21, 2023

@jaka-logar @matthiaslischka please confirm the fix. https://ci.appveyor.com/project/gitextensions/gitextensions/builds/45998786/artifacts

@jaka-logar
Copy link

@jaka-logar @matthiaslischka please confirm the fix. https://ci.appveyor.com/project/gitextensions/gitextensions/builds/45998786/artifacts

I installed updated version but it is not fixed. It is the same behavior. It does not delete any branch.

2023-01-22 21_56_14-Windows Sandbox

Environment info just for sure.

  • Git Extensions 4.1.0.16136
  • Build eff9339 (Dirty)
  • Git 2.39.1.windows.1
  • Microsoft Windows NT 10.0.22621.0
  • .NET 6.0.13
  • DPI 96dpi (no scaling)
  • Microsoft.WindowsDesktop.App Versions
    Microsoft.WindowsDesktop.App 6.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
    Microsoft.WindowsDesktop.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

@matthiaslischka
Copy link
Contributor

You sure you started the correct version? I run the portable and the fix did work just fine. 👍
I also debugged the code on Friday too and came to the same code change.

@pmiossec
Copy link
Member Author

I also debugged the code on Friday too and came to the same code change.

@matthiaslischka So, can you do the code review and approve the PR?
I will merge it after.

I installed updated version but it is not fixed. It is the same behavior.

As I wrote in the PR description, any bad exit code, because the plug-in does not handle the exception well, could end up with this state instead of providing feedback to the user.

If you are working with this plug-in @matthiaslischka, could you have a look at that?

@pmiossec pmiossec merged commit d97d6df into gitextensions:master Jan 23, 2023
@pmiossec pmiossec deleted the fix_delete_unuse_branch_plugin branch January 23, 2023 08:44
@ghost ghost added this to the vNext milestone Jan 23, 2023
@pmiossec pmiossec mentioned this pull request Apr 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RussKie RussKie RussKie approved these changes

@matthiaslischka matthiaslischka matthiaslischka approved these changes

Assignees

@pmiossec pmiossec

Labels
None yet
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.

Delete obsolete branches plugin - deleting stuck
4 participants
@pmiossec @RussKie @jaka-logar @matthiaslischka