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.

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

Add import aliases for packages whose name doesn't match import path #22

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

zchee
Copy link
Contributor

@zchee zchee commented May 26, 2023

For code readability.

@jhump
Copy link
Member

jhump commented May 26, 2023

Hi, @zchee, thanks for opening a pull request!

But I'm afraid the existing gci configuration is intentional. The current import grouping (just two groups) is consistent with other bufbuild repos (like bufbuild/buf and bufbuild/connect-go).

@zchee zchee changed the title all: run goimports happy all: add import alias for not matched import path and package name May 27, 2023
@zchee
Copy link
Contributor Author

zchee commented May 27, 2023

@jhump Ah, I see. That's the bufbuild code style. I respect it.

So, I changed PR about all: add import alias for not matched import path and package name, WDYT?

@zchee
Copy link
Contributor Author

zchee commented Jun 6, 2023

I think import alias also not bufbuild style. Close.

@zchee zchee closed this Jun 6, 2023
@zchee zchee deleted the goimports branch June 6, 2023 03:00
@jhump
Copy link
Member

jhump commented Jun 6, 2023

@zchee, sorry I didn't reply to this. I got pulled into something late last week and have been kind of heads down for a while. I was still considering this pull request. My main concern with merging it is that it won't last very long: as the code is edited and new imports are added, things will change and the aliases may be lost.

I think the intent is completely reasonable. But I also think we need some way to check/enforce it in CI if we want these aliases to be preserved over time. If there's a way to make golangci-lint enforce it, I'd be receptive to it.

@zchee
Copy link
Contributor Author

zchee commented Jun 8, 2023

@jhump Nop!
and yes, golangci-lint has importas linter, which enforce strictly them.

I'll re-open PR with that later, Thanks!

@zchee zchee reopened this Jun 8, 2023
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
Copy link
Contributor Author

zchee commented Jun 8, 2023

@jhump PTAL

Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@jhump jhump changed the title all: add import alias for not matched import path and package name Add import aliases for packages whose name doesn't match import path Jun 8, 2023
@zchee
Copy link
Contributor Author

zchee commented Jun 8, 2023

@jhump Thanks!!

@zchee
Copy link
Contributor Author

zchee commented Jun 8, 2023

@jhump Would you like to send this change to another repository by me?

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

Successfully merging this pull request may close these issues.

None yet

2 participants