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

Also generate import-style targets with Gazelle #1173

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Jun 14, 2023

Since the repo uses checked-in build files, it should support both the import and go_default_library naming conventions supported by Gazelle to ensure that it works with all possible values of the gazelle:go_naming_convention_external directive.

This is achieved by using gazelle:go_naming_convention import_alias.

Fixes bazel-contrib/bazel-gazelle#1557

Since the repo uses checked-in build files, it should support both the
`import` and `go_default_library` naming conventions supported by
Gazelle to ensure that it works with all possible values of the
`gazelle:go_naming_convention_external` directive.

This is achieved by using `gazelle:go_naming_convention import_alias`.
@fmeum
Copy link
Contributor Author

fmeum commented Jun 14, 2023

CC @cgrindel

@vladmos
Copy link
Member

vladmos commented Jul 12, 2023

Former 'go_default_test` rules don't have aliases, is that expected? Is that not part of the convention anymore?

@fmeum
Copy link
Contributor Author

fmeum commented Jul 12, 2023

I'm not entirely sure whether this changed at some point, but as far as I understood the convention the aliases exist just to ensure references from other repos to this repo aren't broken. Other repos would not reference tests, so aliases for them shouldn't be needed.

@fmeum
Copy link
Contributor Author

fmeum commented Aug 19, 2023

@vladmos Friendly ping on this, it would definitely make downstream users' lives easier. :⁠-⁠)

Copy link
Member

@vladmos vladmos left a comment

Choose a reason for hiding this comment

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

Thanks!

@vladmos vladmos merged commit 4f43986 into bazelbuild:master Aug 21, 2023
@vladmos
Copy link
Member

vladmos commented Aug 21, 2023

@fmeum Do you need a new release with these changes or just having them in the repo enough for now?

@fmeum
Copy link
Contributor Author

fmeum commented Aug 21, 2023

@vladmos This particular change is fine without a release - in fact, Go doesn't see the releases since their major versions don't match the import path.

@fmeum fmeum deleted the import-alias branch August 25, 2023 13:45
apattidb pushed a commit to databricks/buildtools that referenced this pull request May 10, 2024
Since the repo uses checked-in build files, it should support both the
`import` and `go_default_library` naming conventions supported by
Gazelle to ensure that it works with all possible values of the
`gazelle:go_naming_convention_external` directive.

This is achieved by using `gazelle:go_naming_convention import_alias`.
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.

Gazelle is not respecting the go_naming_convention directive for external dependencies
2 participants