lint: sort imports #17131
lint: sort imports #17131
Conversation
|
release-note/ignore-for-release |
Codecov Report
@@ Coverage Diff @@
## main #17131 +/- ##
==========================================
- Coverage 67.28% 67.04% -0.24%
==========================================
Files 992 993 +1
Lines 83942 83550 -392
Branches 2665 2665
==========================================
- Hits 56479 56020 -459
- Misses 23584 23653 +69
+ Partials 3879 3877 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
wy65701436
added
the
release-note/ignore-for-release
|
@daixiang0 Thank you for the contribution to Harbor project.
and sort the import within each group. for example: |
|
Sure, the GCI would this work. |
|
Hi @daixiang0 , do we have any update on this PR? |
|
Will update later, thanks. |
|
Now linter only supports:
@zyyw I will update if ok. |
@daixiang0 It will also be grouped into 3 parts, right? |
|
Sure. |
| "fmt" | ||
| "strconv" | ||
| ) |
There was a problem hiding this comment.
This close parentheses ) should NOT be removed. This is important.
There was a problem hiding this comment.
Seems linter's bug, let me fix it.
src/chartserver/handler_repo_test.go
Outdated
| @@ -1,7 +1,6 @@ | |||
| package chartserver | |||
|
|
|||
| import "testing" | |||
|
|
|||
There was a problem hiding this comment.
Why removing this line 4 here? Is it deleted by make lint? Maybe we should keep it.
There was a problem hiding this comment.
Essentially, we would like there is an empty line between import and func TestGetIndexFile. If this line 4 is deleted automatically by make lint, maybe we can try to change
import "testing"
// Test get /index.yaml
func TestGetIndexFile(t *testing.T) {
...
}
to
import (
"testing"
)
// Test get /index.yaml
func TestGetIndexFile(t *testing.T) {
...
}
to ensure there is an empty line between import and its subsequent code.
This is optional. if you prefer to what it is, I'm also okay with it. And it should be consistent, for example, if we change it to be formatted as I suggest, then this kind of one line import in other files should also be updated like this one. Or if you prefer to what it is, then this kind of one line import should also stay the same format as this one.
There was a problem hiding this comment.
Got it, I will try to update.
There was a problem hiding this comment.
Now all linters do not config the one-line import, they will keep it as they were. I recommend that we keep the same with them.
The emply line is necessary, I will update them all but keep one-line import as they were.
| @@ -15,7 +15,6 @@ | |||
| package event | |||
|
|
|||
| import "github.com/goharbor/harbor/src/pkg/reg/model" | |||
|
|
|||
There was a problem hiding this comment.
similar comment to this one:
src/controller/event/model/event.go
Outdated
| @@ -1,7 +1,6 @@ | |||
| package model | |||
|
|
|||
| import "github.com/goharbor/harbor/src/pkg/retention/policy/rule" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
src/controller/jobservice/model.go
Outdated
| @@ -15,7 +15,6 @@ | |||
| package jobservice | |||
|
|
|||
| import "time" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -15,7 +15,6 @@ | |||
| package flow | |||
|
|
|||
| import "github.com/goharbor/harbor/src/pkg/reg/adapter" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -1,7 +1,6 @@ | |||
| package notifications | |||
|
|
|||
| import "github.com/goharbor/harbor/src/core/api" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -15,7 +15,6 @@ | |||
| package legacy | |||
|
|
|||
| import "github.com/goharbor/harbor/src/pkg/scheduler" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -1,7 +1,6 @@ | |||
| package backend | |||
|
|
|||
| import "testing" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -3,7 +3,6 @@ | |||
| package period | |||
|
|
|||
| import mock "github.com/stretchr/testify/mock" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
src/lib/authorizer.go
Outdated
| @@ -15,6 +15,5 @@ | |||
| package lib | |||
|
|
|||
| import "github.com/goharbor/harbor/src/common/http/modifier" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -15,7 +15,6 @@ | |||
| package metadata | |||
|
|
|||
| import "github.com/goharbor/harbor/src/common" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -1,7 +1,6 @@ | |||
| package repository | |||
|
|
|||
| import "net/url" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
src/lib/orm/creator.go
Outdated
| @@ -15,7 +15,6 @@ | |||
| package orm | |||
|
|
|||
| import "github.com/beego/beego/orm" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
src/lib/response_recorder.go
Outdated
| @@ -15,7 +15,6 @@ | |||
| package lib | |||
|
|
|||
| import "net/http" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
src/pkg/retention/models.go
Outdated
| @@ -15,7 +15,6 @@ | |||
| package retention | |||
|
|
|||
| import "time" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -1,7 +1,6 @@ | |||
| package export | |||
|
|
|||
| import "github.com/bmatcuk/doublestar" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -15,7 +15,6 @@ | |||
| package model | |||
|
|
|||
| import "github.com/theupdateframework/notary/tuf/data" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
| @@ -1,7 +1,6 @@ | |||
| package immutable | |||
|
|
|||
| import "fmt" | |||
|
|
|||
There was a problem hiding this comment.
same comment as this one:
Thank you! When you update the code, could you please rebase the latest main into your branch to resolve the conflicts as well? |
|
@zyyw I have rebased the latest main. Thanks for your hard review, the changes are big while meaningful, after this, we can get clear and ordered imports. |
src/common/http/transport.go
Outdated
| "net" | ||
| "net/http" | ||
| "time" | ||
|
|
||
| "github.com/goharbor/harbor/src/lib/trace" |
There was a problem hiding this comment.
line 23 & line 27 are the same.
| "net/http" | ||
| "time" | ||
|
|
||
| "github.com/goharbor/harbor/src/lib/cache/memory" | ||
|
|
There was a problem hiding this comment.
line 23 and line 26 are the same
| "time" | ||
|
|
||
| proModels "github.com/goharbor/harbor/src/pkg/project/models" |
There was a problem hiding this comment.
line 6 and line 11 are the same
|
Goimports do not remove duplicate imports, let me fix it by hand :( |
|
Hi @daixiang0, I'm sorry, maybe we should stay on [email protected], since [email protected] does not works well. Here is my reasons:
|
|
OK, I will turn back to 1.45.2. |
Signed-off-by: Loong Dai <[email protected]>
|
The root cause of CI failure is |
My initial idea is to change the following content in Makefile to But I need to discuss it with the team first. Apart from the Hi @daixiang0 , could you please help to resolve this |
Signed-off-by: Loong Dai <[email protected]>
|
Has fixed, |
|
Thank you for the fix. We can ignore all files under |
Signed-off-by: Loong Dai <[email protected]>
* lint: add goimports Signed-off-by: Loong Dai <[email protected]>
* lint: add goimports Signed-off-by: Loong Dai <[email protected]> Signed-off-by: Stephan Hohn <[email protected]>
Signed-off-by: Loong Dai [email protected]
Thank you for contributing to Harbor!
Comprehensive Summary of your change
Add goimprots linter to group Go imports.
Please indicate you've done the following: