Bugzilla – Full Text Bug Listing
| Summary: | [server][tag] Tagging with a tag name that already exist should not throw 500 error | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Tomasz Zarna <tomasz.zarna> |
| Component: | Git | Assignee: | Tomasz Zarna <tomasz.zarna> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | janikpiotrek, Szymon.Brandys |
| Version: | 0.2 | Flags: | tomasz.zarna:
review+
|
| Target Milestone: | 0.3 M1 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | gsoc2011 | ||
|
Description
Tomasz Zarna
https://github.com/pjanik/orion.server/tree/bug346285 I wrote all this code and have the rights to contribute it to Eclipse under the eclipse.org web site terms of use. Forced update solves all the problems. 500 is not thrown and it also moves a tag when it's needed. Nice try, but this is not what we want ;) When tagging the user will not be aware that he's replacing an existing tag. Moreover, he needs to refresh the page to figure out which commit he has just tagged. What we need here is a user friendly message saying why tagging failed. This makes this bug somehow similar to bug 353135. So, do you want to move tag or return error message? I can see some inconsistency. ;) When did I say I want to move the tag? What I meant in comment 0 is that both actions (tagging a commit with a tag which was already used to on that commit and tagging a different commit with an existing tag) should have the same output. An output different than 500. Updated. https://github.com/pjanik/orion.server/tree/bug346285 https://github.com/pjanik/orion.client/tree/bug346285 500 is still thrown as this action causes JGitInternalException and I cannot distinguish a cause (i.e. that tag already exist etc.). It's JGit specific problem. I've added in client showing error on the status panel. Now, detailed message from JGit is show (it isn't beautiful). Other option is to show just "An error occured when tagging.", but we can miss some useful information in other error case, when JGitInternalException is thrown and detailed message is more descriptive. Do we have a test case covering this? Also, once you've modified GitCommitHandlerV1.tag(HttpServletRequest, HttpServletResponse, Repository, String, String, boolean) I think you should do the same with GitTagHandlerV1.handlePost(HttpServletRequest, HttpServletResponse, String) even though it's not called by the client. Test cases for both ways of tagging would be required to make sure they both fail in the same manner. (In reply to comment #6) > GitCommitHandlerV1.tag(HttpServletRequest, HttpServletResponse, Repository, String, String, boolean) btw, the signature can be cleaned up in terms of thrown exceptions. Fixed, test added. |