fix variable assigned and not used. #173
Conversation
appleboy
force-pushed
the
ineffassign98
branch
from
7625913 to
52009c7
Compare
Current coverage is 3.03% (diff: 0.00%)@@ master #173 diff @@
========================================
Files 33 33
Lines 8075 8096 +21
Methods 0 0
Messages 0 0
Branches 0 0
========================================
Hits 246 246
- Misses 7809 7830 +21
Partials 20 20
|
|
This is really need a test. |
|
@lunny There is no testing before make this PR. I just fix some error from goreport card service. |
| fmt.Sprintf("ForkRepository(git clone): %s/%s", u.Name, repo.Name), | ||
| "git", "clone", "--bare", oldRepo.RepoPath(), repoPath) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("git clone: %v", stderr) | ||
| return nil, fmt.Errorf("git clone: %v", err) |
There was a problem hiding this comment.
Here you have changed the functionality
| @@ -2088,14 +2088,14 @@ func ForkRepository(u *User, oldRepo *Repository, name, desc string) (_ *Reposit | |||
| } | |||
|
|
|||
| repoPath := RepoPath(u.Name, repo.Name) | |||
| _, stderr, err := process.ExecTimeout(10*time.Minute, | |||
| _, _, err = process.ExecTimeout(10*time.Minute, | |||
Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
appleboy
force-pushed
the
ineffassign98
branch
from
52009c7 to
d9ffe99
Compare
|
@tboerger I changed |
| @@ -2099,7 +2099,7 @@ func ForkRepository(u *User, oldRepo *Repository, name, desc string) (_ *Reposit | |||
| repoPath, fmt.Sprintf("ForkRepository(git update-server-info): %s", repoPath), | |||
| "git", "update-server-info") | |||
| if err != nil { | |||
| return nil, fmt.Errorf("git update-server-info: %v", err) | |||
| return nil, fmt.Errorf("git update-server-info: %v", stderr) | |||
There was a problem hiding this comment.
The condition to print is err being not nil, so why printing stderr instead ?
There was a problem hiding this comment.
Figured, it's just process.ExecDir in modules/process/manager.go that would need a somewhat extended documentation. All returns are in the form:
- return "", err.Error(), err
- return "", ErrExecTimeout.Error(), ErrExecTimeout
- return bufOut.String(), bufErr.String(), err
where the err variable in the last case is a return from cmd.Wait (exec.ExitError) while bufErr.String() is the stderr.
So the signature of ExecDir has a duplicated return value, because the "stderr" part is always included in the "err".
I suggest ExecDir signature is changed, to reduce confusion.
| @@ -2099,7 +2099,7 @@ func ForkRepository(u *User, oldRepo *Repository, name, desc string) (_ *Reposit | |||
| repoPath, fmt.Sprintf("ForkRepository(git update-server-info): %s", repoPath), | |||
| "git", "update-server-info") | |||
| if err != nil { | |||
| return nil, fmt.Errorf("git update-server-info: %v", err) | |||
| return nil, fmt.Errorf("git update-server-info: %v", stderr) | |||
There was a problem hiding this comment.
Figured, it's just process.ExecDir in modules/process/manager.go that would need a somewhat extended documentation. All returns are in the form:
- return "", err.Error(), err
- return "", ErrExecTimeout.Error(), ErrExecTimeout
- return bufOut.String(), bufErr.String(), err
where the err variable in the last case is a return from cmd.Wait (exec.ExitError) while bufErr.String() is the stderr.
So the signature of ExecDir has a duplicated return value, because the "stderr" part is always included in the "err".
I suggest ExecDir signature is changed, to reduce confusion.
|
Refactoring is not part of this pr, I'm fine with this change. LGTM |
|
Ok agreed, let's deal with refactoring somewhere else. LGTM. |
…gitea#174) * Restores the HashTag code in Issue comments * Has travis test with 1.7 and 1.8 of Go
Remove some variable assigned and not used.
ref: https://goreportcard.com/report/github.com/go-gitea/gitea#ineffassign
Signed-off-by: Bo-Yi Wu appleboy.tw@gmail.com