Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

all: remove duplicate imports #2277

Merged
merged 6 commits into from
Aug 11, 2021
Merged

all: remove duplicate imports #2277

merged 6 commits into from
Aug 11, 2021

Conversation

zchee
Copy link
Contributor

@zchee zchee commented Jul 25, 2021

all: remove duplicate imports.

@zchee
Copy link
Contributor Author

zchee commented Jul 25, 2021

/cc @tonistiigi

@zchee zchee force-pushed the remove-dupimport branch from 613ca65 to 1ca460b Compare July 25, 2021 07:32
@zchee
Copy link
Contributor Author

zchee commented Jul 26, 2021 •
edited
Loading

@thaJeztah cc: @AkihiroSuda @crazy-max

As @thaJeztah suggested, I'd choiced and unified the specs-go package import alias to ocispecs. Noted also in the commit message, which means "O" pen "C" ontainer "I" nitiative image-spec/ "specs" -go/v1.
Because shorthand specs conflicted github.com/opencontainers/image-spec/specs-go such as https://github.com/moby/buildkit/pull/2277/files#diff-ee35853b147517e9f5e781474c10ed66da6990a6a7c6813338e5cfea8e68d3f9R20.

Also, upgrade the golangci-lint version to v1.41.1 to use importas linter. We can setting and validation any import aliases. such as go-digest package, See also zchee/moby-buildkit@remove-dupimport...strict-digest-import-alias.

PTAL.

Note: Currently, golangci-lint outputs a warning message which "golint has been deprecated, use revive instead". But if enabled this linter, faced some lint error a bit. So let's ignore the warning, leave to use golint for now.
If you certainly want to fix the revive lint error, I'll send another PR later.

Ref: zchee/moby-buildkit@remove-dupimport...remove-dupimport-revive

zchee
zchee commented Jul 26, 2021
View reviewed changes
.golangci.yml
- goimports
- golint
- govet
- importas
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add importas and sorted alphabetically.

@zchee
Copy link
Contributor Author

zchee commented Jul 30, 2021

@thaJeztah @AkihiroSuda @crazy-max
Sorry for mentioned.
I know your busy... but have any opinions?

@tonistiigi
Copy link
Member

Needs rebase

zchee added 6 commits August 11, 2021 08:28
@zchee
all: remove duplicate imports
14a1d6e 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
all: unify the specs-go package import alias to ocispecs
671ae38 
ocispecs means "O"pen "C"ontainer "I"nitiative image-spec/"specs"-go/v1
                      opencontainers          /image-spec/specs-go/v1

Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
hack/dockerfiles: upgrade golangci-lint version to v1.41.1
9cc9d6c 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
golangci-lint: enable importas and add settings for specs-go package
414772d 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
all: unify the go-digest package import alias to digest
50ae0d8 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
golangci-lint: add go-digest importas setting
9e6ea26 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee zchee force-pushed the remove-dupimport branch from 0e02981 to 9e6ea26 Compare August 10, 2021 23:29
@zchee
Copy link
Contributor Author

zchee commented Aug 10, 2021

@tonistiigi done

@tonistiigi tonistiigi merged commit 175e841 into moby:master Aug 11, 2021
@zchee
Copy link
Contributor Author

zchee commented Aug 11, 2021

@tonistiigi Thanks!

@zchee zchee deleted the remove-dupimport branch August 11, 2021 06:32
@thaJeztah
Copy link
Member

Sorry for mentioned.
I know your busy... but have any opinions?

oh! looks like I missed your comment; LGTM, thanks!

@thaJeztah thaJeztah mentioned this pull request Sep 4, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaJeztah thaJeztah thaJeztah left review comments

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@crazy-max crazy-max crazy-max approved these changes

@tonistiigi tonistiigi tonistiigi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zchee @tonistiigi @thaJeztah @crazy-max @AkihiroSuda