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

Handle errors accessing the amend state file #9037

Merged
1 commit merged into from Mar 29, 2021

Conversation

mstv
Copy link
Member

@mstv mstv commented Mar 26, 2021

Fixes #9036

Proposed changes

  • Handle errors accessing the amend state file the same way as for the commit message file

Screenshots

N/A

Test methodology

  • manual
  • slightly adapted existing NUnit tests

Test environment(s)

  • Git Extensions 33.33.33
  • Build 28d6627
  • Git 2.27.0.windows.1 (recommended: 2.30.0 or later)
  • Microsoft Windows NT 10.0.19042.0
  • .NET Framework 4.8.4300.0
  • DPI 96dpi (no scaling)

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

@mstv mstv self-assigned this Mar 26, 2021
@mstv mstv marked this pull request as draft March 26, 2021 20:54
}
catch (Exception ex)
{
MessageBox.Show(null, string.Format(CannotAccessFile, ex.Message, filePath), errorTitle,
Copy link
Member

Choose a reason for hiding this comment

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

Let it bubble up to the global handler?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I agree to what you wrote "in this specific scenario we need to ignore the exception".
Just let the user know that something went wrong but I see no need to cancel the other operations in FormCommit.

Copy link
Member Author

Choose a reason for hiding this comment

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

(If these file accesses would fail often, a retry should be offered here.)

Copy link
Member

Choose a reason for hiding this comment

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

I think we should leave a comment here explaining why we're swallowing the exception and not sending to the global handler.

@mstv mstv marked this pull request as ready for review March 26, 2021 21:31
@mstv
Copy link
Member Author

mstv commented Mar 28, 2021

@msftbot merge in 1 day

@ghost ghost added the status: auto merge label Mar 28, 2021
@ghost
Copy link

ghost commented Mar 28, 2021

Hello @mstv!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Mon, 29 Mar 2021 20:50:38 GMT, which is in 1 day

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

the same way as for the commit message
@ghost ghost merged commit 99f384b into gitextensions:master Mar 29, 2021
@ghost ghost added this to the 3.6 milestone Mar 29, 2021
@mstv mstv deleted the fix/9036_delete_amend branch March 31, 2021 15:56
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NBug] Не удалось найти часть пути "E:\test_screenshots\.git\GitEx...
2 participants