JGit: Propagate exceptions when pushing tags/branches #1271
Conversation
91becec to
fcc8bd3
Compare
Add tests for all Git providers for failing push operations This closes #1253
fcc8bd3 to
dc9ee71
Compare
|
|
||
| /** | ||
| * Exception thrown when a push operation fails in JGit. | ||
| * This is a custom exception that extends GitAPIException to provide more context |
There was a problem hiding this comment.
It doesn't extend GitAPIException
There was a problem hiding this comment.
Will fix it, used to extend it, but it does no longer because it was not necessary/useful.
|
|
||
| } catch (PushException e) { | ||
| return new BranchScmResult("JGit branch", "Failed to push changes: " + e.getMessage(), "", false); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
can this be a more specific exception?
There was a problem hiding this comment.
For me this is already as specific as it can get. What do you suggest here?
There was a problem hiding this comment.
Is something in the try block throwing a raw java.lang.Exception? It shouldn't be, but it isn't unheard of for APIs to make that mistake
There was a problem hiding this comment.
line 136 was not touched in this PR and is not related. This can be done in some other PR!
There was a problem hiding this comment.
Line 136 is related. You've changed the exceptions thrown in the try block. That suggests it's worth looking at the catch blocks to make sure they're still needed.
There was a problem hiding this comment.
I changed only one potential cause of exceptions. The try block involves lots of other JGit commands, therefore I don't want to change this in this context.
| return new CheckInScmResult("JGit checkin", checkedInFiles); | ||
| } catch (PushException e) { | ||
| return new CheckInScmResult("JGit checkin", "Failed to push changes: " + e.getMessage(), "", false); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
can this now be more specific?
369981b to
32c5eb9
Compare
|
@kwin Please assign appropriate label to PR according to the type of change. |
Add tests for all Git providers for failing push operations This closes #1253
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
mvn -Prun-its verify).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.