-
Notifications
You must be signed in to change notification settings - Fork 38
Fix for bad commitID to show up in error #148
Fix for bad commitID to show up in error #148
Conversation
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
==========================================
+ Coverage 36.07% 36.48% +0.41%
==========================================
Files 28 28
Lines 1838 1839 +1
==========================================
+ Hits 663 671 +8
+ Misses 1085 1079 -6
+ Partials 90 89 -1
Continue to review full report at Codecov.
|
@@ -153,13 +153,14 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) { | |||
func (repo *Repository) GetCommit(commitID string) (*Commit, error) { | |||
if len(commitID) != 40 { | |||
var err error | |||
commitID, err = NewCommand("rev-parse", commitID).RunInDir(repo.Path) | |||
actualCommitID, err := NewCommand("rev-parse", commitID).RunInDir(repo.Path) | |||
if err != nil { | |||
if strings.Contains(err.Error(), "unknown revision or path") { | |||
return nil, ErrNotExist{commitID, ""} |
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.
Here is where commitID was always empty because NewCommand() returned nil for it, replacing the commitID passed to the function.
Thanks for another PR @richmahn 😄 Now that this has been merged would you be able to update the deps in the main repo? Please let me know if you have any questions. |
@techknowlogick Yes, will do that. |
This error message for a bad commitID makes no sense because when the Error object is created, commitID is always empty since NewCommand() call replaced the given commitID. This makes sure we retain the old commitID until after this block.