8000 feat(gazelle)!: Move the plugin to a separate workspace by aignas · Pull Request #972 · bazel-contrib/rules_python · GitHub
[go: up one dir, main page]
Skip to content

feat(gazelle)!: Move the plugin to a separate workspace #972

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
Merged
merged 1 commit into from
Jan 25, 2023
Merged

feat(gazelle)!: Move the plugin to a separate workspace #972

merged 1 commit into from
Jan 25, 2023

Conversation

aignas
Copy link
Collaborator
@aignas aignas commented Jan 4, 2023 •
edited
Loading

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Previously the gazelle plugin was part of the same WORKSPACE. This means that we cannot have a separate bzlmod module and make the rules_python repo work with both, bzlmod and legacy dependency management systems.

Issue Number: #965

What is the new behavior?

The gazelle plugin is now isolated with its own WORKSPACE file. Whilst making this I have also moved the plugin source code to a separate directory.

Summary:

  • Move go.mod to gazelle.
  • Move gazelle definition.
  • Move the gazelle plugin to a separate folder, just like bazel-skylib does, which helps with naming of the externally visible targets. This is now refactor(gazelle): Move plugin to a separate directory. #983.
  • Fix file distribution for the gazelle module.
  • Update the example test.
  • Include rules_python_gazelle_plugin during integration tests
  • Update ignored packages

Does this PR introduce a breaking change?

  • Yes
  • No

Steps that need to be taken:

  • Add a new dependency to your WORKSPACE and change the import path for the plugin dependency setup.
  • If you are using the gazelle binary from the plugin repo, use the new label @rules_python_gazelle_plugin//python:gazelle_binary.
  • If you are using the gazelle language extension, use the new label @rules_python_gazelle_plugin//python.
  • gazelle_python.yaml integrity gets changed for all users.

@aignas aignas mentioned this pull request Jan 4, 2023
Closed
12 tasks
@aignas aignas force-pushed the gazelle-bzlmod-5 branch 4 times, most recently from 5ae27f4 to abde25c Compare January 4, 2023 10:51
@aignas aignas marked this pull request as ready for review January 4, 2023 11:01
@aignas aignas mentioned this pull request Jan 4, 2023
Closed
chrislovecnm
chrislovecnm suggested changes Jan 4, 2023
View reviewed changes
Copy link
Contributor
@chrislovecnm chrislovecnm left a comment

Choose a reason for hiding this comment

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

Look great. I have some nitpicks that I noticed, but I like it!! Working on the build_file_generation example I was wondering if bzmod and gazelle where working.

@aignas aignas force-pushed the gazelle-bzlmod-5 branch 2 times, most recently from 7b0ec08 to 2ad688f Compare January 5, 2023 08:03
@chrislovecnm
Copy link
Contributor

@aignas LGTM. Since this is a breaking change, do we need release notes? @f0rmiga can you PTLA?

chrislovecnm
chrislovecnm approved these changes Jan 10, 2023
View reviewed changes
Copy link
Contributor
@chrislovecnm chrislovecnm left a comment

Choose a reason for hiding this comment

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

LGTM

@alexeagle
Copy link
Contributor

FYI @f0rmiga is on paternity leave, back next week I believe

@chrislovecnm
Copy link
Contributor

@alexeagle who else can review it?

@alexeagle
Copy link
Contributor
alexeagle commented Jan 11, 2023 •
edited
Loading

I think we should probably just wait for @f0rmiga to be back. This will need someone to invest a few hours.

As one way to make this easier to land, @aignas is it possible to split out a separate PR that does the directory rename so this doesn't show as a 433 files changed? Or leave the directory rename for a subsequent refactoring? I can't even get started on reviewing it at this size without setting aside a chunk of my day.

(Also, thank you for taking this on!!)

@aignas
Copy link
Collaborator Author
aignas commented Jan 12, 2023 •
edited
Loading

Hey @alexeagle, I also think that waiting for @f0rmiga is probably for the best. I can split it to several commits at the very least. I think the rename is necessary to make everything work as we are moving the gazelle rule usage to the gazelle folder and it makes things a little bit tricky.

What I can do to make it easier to review is to split it into multiple PRs introducing a breaking change:

  • Do the folder rename.
  • Add a new WORKSPACE.
  • Whatever is left afterwards.

How does it sound to you? And sorry for making the initial PR too long, I was mainly looking at the changed lines counter.

@alexeagle
Copy link
Contributor

Yeah I think the folder rename first will make it more manageable. Thanks!

@aignas aignas mentioned this pull request Jan 12, 2023
Merged
12 tasks
aignas added a commit to aignas/rules_python that referenced this pull request Jan 12, 2023
@aignas
refactor(gazelle): Move plugin to a separate directory.
75f8f60 
This is in order to make bazel-contrib#972 easier to review. This PR is only moving
files and addressing a few small review comments made in the initial
review of bazel-contrib#972.

Work towards bazel-contrib#965.
aignas added a commit to aignas/rules_python that referenced this pull request Jan 12, 2023
@aignas
refactor(gazelle): Move plugin to a separate directory.
6257e87 
This is in order to make bazel-contrib#972 easier to review. This PR is only moving
files and addressing a few small review comments made in the initial
review of bazel-contrib#972.

Work towards bazel-contrib#965.
aignas added a commit to aignas/rules_python that referenced this pull request Jan 12, 2023
@aignas
refactor(gazelle): Move plugin to a separate directory.
b7743a9 
This is in order to make bazel-contrib#972 easier to review. This PR is only moving
files and addressing a few small review comments made in the initial
review of bazel-contrib#972.

Work towards bazel-contrib#965.
@aignas aignas changed the title breaking: feat(gazelle): Move the plugin to a separate workspace feat(gazelle)!: Move the plugin to a separate workspace Jan 12, 2023
aignas added a commit to aignas/rules_python that referenced this pull request Jan 24, 2023
@aignas
refactor(gazelle): Move plugin to a separate directory.
44398ef 
This is in order to make bazel-contrib#972 easier to review. This PR is only moving
files and addressing a few small review comments made in the initial
review of bazel-contrib#972.

Work towards bazel-contrib#965.
@aignas aignas force-pushed the gazelle-bzlmod-5 branch 2 times, most recently from d1de560 to d9bd232 Compare January 24, 2023 06:31
@aignas
Copy link
Collaborator Author
aignas commented Jan 24, 2023

@f0rmiga, I think once you merge #983, this PR should be much more approachable.

@aignas
feat!(gazelle): Move the plugin to a separate workspace
dc0db9d 
Summary:
* Move go.mod to gazelle.
* Move gazelle definition.
* Fix file distribution for the gazelle module.
* Update the example test.
* Include rules_python_gazelle_plugin during integration tests
* Update ignored packages
* Update CI configuration
@f0rmiga f0rmiga merged commit fd5f531 into bazel-contrib:main Jan 25, 2023
@f0rmiga
Copy link
Member
f0rmiga commented Jan 25, 2023

Thanks for addressing this, @aignas!

@aignas aignas deleted the gazelle-bzlmod-5 branch January 25, 2023 22:39
@renovate renovate bot mentioned this pull request Jun 19, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrislovecnm chrislovecnm chrislovecnm approved these changes

@f0rmiga f0rmiga f0rmiga approved these changes

@rickeylev rickeylev Awaiting requested review from rickeylev rickeylev is a code owner

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

Successfully merging this pull request may close these issues.
4 participants
@aignas @chrislovecnm @alexeagle @f0rmiga
0