Skip to content
This repository was archived by the owner on Apr 12, 2019. It is now read-only.

Fix for bad commitID to show up in error #148

Conversation

richmahn
Copy link

@richmahn richmahn commented Mar 6, 2019

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.

@codecov-io
Copy link

codecov-io commented Mar 6, 2019

Codecov Report

Merging #148 into master will increase coverage by 0.41%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
repo_commit.go 28.67% <100%> (+2.84%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8983773...0c7c947. Read the comment docs.

@@ -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, ""}
Copy link
Author

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.

@techknowlogick techknowlogick merged commit 6cba05a into go-gitea:master Mar 8, 2019
@techknowlogick
Copy link
Member

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.

@richmahn
Copy link
Author

richmahn commented Mar 8, 2019

@techknowlogick Yes, will do that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants