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

Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged

feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom
aignas:gazelle-bzlmod-5

Conversation

@aignas

@aignasaignas commented Jan 4, 2023•
edited
Loading

Copy link
Copy Markdown
Collaborator

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.

@aignasaignas mentioned this pull request Jan 4, 2023
Closed
12 tasks
@aignas
aignasforce-pushed the gazelle-bzlmod-5 branch 4 times, most recently from 5ae27f4 to abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas marked this pull request as ready for review January 4, 2023 11:01
@aignasaignas mentioned this pull request Jan 4, 2023
Closed
chrislovecnm
chrislovecnm suggested changes Jan 4, 2023
View reviewed changes

@chrislovecnmchrislovecnm left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread.bazelrc Outdated
Comment threadexamples/build_file_generation/WORKSPACE Outdated
Comment threadgazelle/.gitignore Outdated
Comment threadgazelle/python/python_test.go Outdated
@aignas
aignasforce-pushed the gazelle-bzlmod-5 branch 2 times, most recently from 7b0ec08 to 2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm

chrislovecnm commented Jan 10, 2023

Copy link
Copy Markdown
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

@chrislovecnmchrislovecnm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@alexeagle

alexeagle commented Jan 10, 2023

Copy link
Copy Markdown
Contributor

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

@chrislovecnm

chrislovecnm commented Jan 10, 2023

Copy link
Copy Markdown
Contributor

@alexeagle who else can review it?

@alexeagle

alexeagle commented Jan 11, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

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

aignas commented Jan 12, 2023•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

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

alexeagle commented Jan 12, 2023

Copy link
Copy Markdown
Contributor

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

@aignasaignas 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.
@aignasaignas changed the title breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed the gazelle-bzlmod-5 branch 2 times, most recently from d1de560 to d9bd232CompareJanuary 24, 2023 06:31
@aignas

aignas commented Jan 24, 2023

Copy link
Copy Markdown
CollaboratorAuthor

@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:mainJan 25, 2023
@f0rmiga

f0rmiga commented Jan 25, 2023

Copy link
Copy Markdown
Member

Thanks for addressing this, @aignas!

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

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner
2 more reviewers
@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
feat(gazelle)!: Move the plugin to a separate workspace - #972
Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged
feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Conversation
@aignas
@aignasaignas
commented
Jan 4, 2023•

edited

Loading

Copy link


Copy Markdown

Collaborator
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.
@aignasaignas
mentioned this pull request
Jan 4, 2023
 Closed

12 tasks
@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
4 times, most recently
from
5ae27f4 to
abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas
marked this pull request as ready for review
January 4, 2023 11:01
@aignasaignas
mentioned this pull request
Jan 4, 2023

Closed

chrislovecnm
chrislovecnm
suggested changes
Jan 4, 2023
View reviewed changes
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
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.
Comment thread.bazelrc
Outdated

Comment threadexamples/build_file_generation/WORKSPACE
Outdated

Comment threadgazelle/.gitignore
Outdated

Comment threadgazelle/python/python_test.go
Outdated

@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
7b0ec08 to
2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

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
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@alexeagle
alexeagle
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
FYI @f0rmiga is on paternity leave, back next week I believe
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
@alexeagle who else can review it?
@alexeagle
alexeagle
commented
Jan 11, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
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
aignas
commented
Jan 12, 2023•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
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
alexeagle
commented
Jan 12, 2023

Copy link


Copy Markdown

Contributor
Yeah I think the folder rename first will make it more manageable. Thanks!
@aignasaignas
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.
@aignasaignas
changed the title
breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
d1de560 to
d9bd232CompareJanuary 24, 2023 06:31
@aignas
aignas
commented
Jan 24, 2023

Copy link


Copy Markdown

CollaboratorAuthor
@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:mainJan 25, 2023
@f0rmiga
f0rmiga
commented
Jan 25, 2023

Copy link


Copy Markdown

Member
Thanks for addressing this, @aignas!
@aignas
aignas
deleted the

gazelle-bzlmod-5

branch
January 25, 2023 22:39
@renovaterenovateBot
mentioned this pull request
Jun 19, 2023
 Merged

1 task
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner

2 more reviewers

@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(gazelle)!: Move the plugin to a separate workspace - #972
Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged
feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Conversation
@aignas
@aignasaignas
commented
Jan 4, 2023•

edited

Loading

Copy link


Copy Markdown

Collaborator
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.
@aignasaignas
mentioned this pull request
Jan 4, 2023
 Closed

12 tasks
@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
4 times, most recently
from
5ae27f4 to
abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas
marked this pull request as ready for review
January 4, 2023 11:01
@aignasaignas
mentioned this pull request
Jan 4, 2023

Closed

chrislovecnm
chrislovecnm
suggested changes
Jan 4, 2023
View reviewed changes
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
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.
Comment thread.bazelrc
Outdated

Comment threadexamples/build_file_generation/WORKSPACE
Outdated

Comment threadgazelle/.gitignore
Outdated

Comment threadgazelle/python/python_test.go
Outdated

@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
7b0ec08 to
2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

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
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@alexeagle
alexeagle
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
FYI @f0rmiga is on paternity leave, back next week I believe
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
@alexeagle who else can review it?
@alexeagle
alexeagle
commented
Jan 11, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
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
aignas
commented
Jan 12, 2023•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
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
alexeagle
commented
Jan 12, 2023

Copy link


Copy Markdown

Contributor
Yeah I think the folder rename first will make it more manageable. Thanks!
@aignasaignas
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.
@aignasaignas
changed the title
breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
d1de560 to
d9bd232CompareJanuary 24, 2023 06:31
@aignas
aignas
commented
Jan 24, 2023

Copy link


Copy Markdown

CollaboratorAuthor
@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:mainJan 25, 2023
@f0rmiga
f0rmiga
commented
Jan 25, 2023

Copy link


Copy Markdown

Member
Thanks for addressing this, @aignas!
@aignas
aignas
deleted the

gazelle-bzlmod-5

branch
January 25, 2023 22:39
@renovaterenovateBot
mentioned this pull request
Jun 19, 2023
 Merged

1 task
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner

2 more reviewers

@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(gazelle)!: Move the plugin to a separate workspace - #972
Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged
feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Conversation
@aignas
@aignasaignas
commented
Jan 4, 2023•

edited

Loading

Copy link


Copy Markdown

Collaborator
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.
@aignasaignas
mentioned this pull request
Jan 4, 2023
 Closed

12 tasks
@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
4 times, most recently
from
5ae27f4 to
abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas
marked this pull request as ready for review
January 4, 2023 11:01
@aignasaignas
mentioned this pull request
Jan 4, 2023

Closed

chrislovecnm
chrislovecnm
suggested changes
Jan 4, 2023
View reviewed changes
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
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.
Comment thread.bazelrc
Outdated

Comment threadexamples/build_file_generation/WORKSPACE
Outdated

Comment threadgazelle/.gitignore
Outdated

Comment threadgazelle/python/python_test.go
Outdated

@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
7b0ec08 to
2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

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
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@alexeagle
alexeagle
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
FYI @f0rmiga is on paternity leave, back next week I believe
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
@alexeagle who else can review it?
@alexeagle
alexeagle
commented
Jan 11, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
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
aignas
commented
Jan 12, 2023•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
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
alexeagle
commented
Jan 12, 2023

Copy link


Copy Markdown

Contributor
Yeah I think the folder rename first will make it more manageable. Thanks!
@aignasaignas
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.
@aignasaignas
changed the title
breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
d1de560 to
d9bd232CompareJanuary 24, 2023 06:31
@aignas
aignas
commented
Jan 24, 2023

Copy link


Copy Markdown

CollaboratorAuthor
@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:mainJan 25, 2023
@f0rmiga
f0rmiga
commented
Jan 25, 2023

Copy link


Copy Markdown

Member
Thanks for addressing this, @aignas!
@aignas
aignas
deleted the

gazelle-bzlmod-5

branch
January 25, 2023 22:39
@renovaterenovateBot
mentioned this pull request
Jun 19, 2023
 Merged

1 task
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner

2 more reviewers

@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
feat(gazelle)!: Move the plugin to a separate workspace - #972
Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged
feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Conversation
@aignas
@aignasaignas
commented
Jan 4, 2023•

edited

Loading

Copy link


Copy Markdown

Collaborator
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.
@aignasaignas
mentioned this pull request
Jan 4, 2023
 Closed

12 tasks
@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
4 times, most recently
from
5ae27f4 to
abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas
marked this pull request as ready for review
January 4, 2023 11:01
@aignasaignas
mentioned this pull request
Jan 4, 2023

Closed

chrislovecnm
chrislovecnm
suggested changes
Jan 4, 2023
View reviewed changes
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
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.
Comment thread.bazelrc
Outdated

Comment threadexamples/build_file_generation/WORKSPACE
Outdated

Comment threadgazelle/.gitignore
Outdated

Comment threadgazelle/python/python_test.go
Outdated

@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
7b0ec08 to
2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

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
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@alexeagle
alexeagle
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
FYI @f0rmiga is on paternity leave, back next week I believe
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
@alexeagle who else can review it?
@alexeagle
alexeagle
commented
Jan 11, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
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
aignas
commented
Jan 12, 2023•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
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
alexeagle
commented
Jan 12, 2023

Copy link


Copy Markdown

Contributor
Yeah I think the folder rename first will make it more manageable. Thanks!
@aignasaignas
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.
@aignasaignas
changed the title
breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
d1de560 to
d9bd232CompareJanuary 24, 2023 06:31
@aignas
aignas
commented
Jan 24, 2023

Copy link


Copy Markdown

CollaboratorAuthor
@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:mainJan 25, 2023
@f0rmiga
f0rmiga
commented
Jan 25, 2023

Copy link


Copy Markdown

Member
Thanks for addressing this, @aignas!
@aignas
aignas
deleted the

gazelle-bzlmod-5

branch
January 25, 2023 22:39
@renovaterenovateBot
mentioned this pull request
Jun 19, 2023
 Merged

1 task
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner

2 more reviewers

@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(gazelle)!: Move the plugin to a separate workspace - #972
Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged
feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Conversation
@aignas
@aignasaignas
commented
Jan 4, 2023•

edited

Loading

Copy link


Copy Markdown

Collaborator
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.
@aignasaignas
mentioned this pull request
Jan 4, 2023
 Closed

12 tasks
@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
4 times, most recently
from
5ae27f4 to
abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas
marked this pull request as ready for review
January 4, 2023 11:01
@aignasaignas
mentioned this pull request
Jan 4, 2023

Closed

chrislovecnm
chrislovecnm
suggested changes
Jan 4, 2023
View reviewed changes
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
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.
Comment thread.bazelrc
Outdated

Comment threadexamples/build_file_generation/WORKSPACE
Outdated

Comment threadgazelle/.gitignore
Outdated

Comment threadgazelle/python/python_test.go
Outdated

@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
7b0ec08 to
2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

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
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@alexeagle
alexeagle
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
FYI @f0rmiga is on paternity leave, back next week I believe
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
@alexeagle who else can review it?
@alexeagle
alexeagle
commented
Jan 11, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
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
aignas
commented
Jan 12, 2023•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
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
alexeagle
commented
Jan 12, 2023

Copy link


Copy Markdown

Contributor
Yeah I think the folder rename first will make it more manageable. Thanks!
@aignasaignas
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.
@aignasaignas
changed the title
breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
d1de560 to
d9bd232CompareJanuary 24, 2023 06:31
@aignas
aignas
commented
Jan 24, 2023

Copy link


Copy Markdown

CollaboratorAuthor
@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:mainJan 25, 2023
@f0rmiga
f0rmiga
commented
Jan 25, 2023

Copy link


Copy Markdown

Member
Thanks for addressing this, @aignas!
@aignas
aignas
deleted the

gazelle-bzlmod-5

branch
January 25, 2023 22:39
@renovaterenovateBot
mentioned this pull request
Jun 19, 2023
 Merged

1 task
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner

2 more reviewers

@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(gazelle)!: Move the plugin to a separate workspace - #972
Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged
feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Conversation
@aignas
@aignasaignas
commented
Jan 4, 2023•

edited

Loading

Copy link


Copy Markdown

Collaborator
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.
@aignasaignas
mentioned this pull request
Jan 4, 2023
 Closed

12 tasks
@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
4 times, most recently
from
5ae27f4 to
abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas
marked this pull request as ready for review
January 4, 2023 11:01
@aignasaignas
mentioned this pull request
Jan 4, 2023

Closed

chrislovecnm
chrislovecnm
suggested changes
Jan 4, 2023
View reviewed changes
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
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.
Comment thread.bazelrc
Outdated

Comment threadexamples/build_file_generation/WORKSPACE
Outdated

Comment threadgazelle/.gitignore
Outdated

Comment threadgazelle/python/python_test.go
Outdated

@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
7b0ec08 to
2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

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
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@alexeagle
alexeagle
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
FYI @f0rmiga is on paternity leave, back next week I believe
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
@alexeagle who else can review it?
@alexeagle
alexeagle
commented
Jan 11, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
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
aignas
commented
Jan 12, 2023•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
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
alexeagle
commented
Jan 12, 2023

Copy link


Copy Markdown

Contributor
Yeah I think the folder rename first will make it more manageable. Thanks!
@aignasaignas
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.
@aignasaignas
changed the title
breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
d1de560 to
d9bd232CompareJanuary 24, 2023 06:31
@aignas
aignas
commented
Jan 24, 2023

Copy link


Copy Markdown

CollaboratorAuthor
@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:mainJan 25, 2023
@f0rmiga
f0rmiga
commented
Jan 25, 2023

Copy link


Copy Markdown

Member
Thanks for addressing this, @aignas!
@aignas
aignas
deleted the

gazelle-bzlmod-5

branch
January 25, 2023 22:39
@renovaterenovateBot
mentioned this pull request
Jun 19, 2023
 Merged

1 task
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner

2 more reviewers

@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
feat(gazelle)!: Move the plugin to a separate workspace - #972
Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Jan 25, 2023
Merged
feat(gazelle)!: Move the plugin to a separate workspace#972
f0rmiga merged 1 commit into
bazel-contrib:mainfrom 
aignas:gazelle-bzlmod-5
Conversation
@aignas
@aignasaignas
commented
Jan 4, 2023•

edited

Loading

Copy link


Copy Markdown

Collaborator
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.
@aignasaignas
mentioned this pull request
Jan 4, 2023
 Closed

12 tasks
@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
4 times, most recently
from
5ae27f4 to
abde25cCompareJanuary 4, 2023 10:51
@aignas
aignas
marked this pull request as ready for review
January 4, 2023 11:01
@aignasaignas
mentioned this pull request
Jan 4, 2023

Closed

chrislovecnm
chrislovecnm
suggested changes
Jan 4, 2023
View reviewed changes
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
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.
Comment thread.bazelrc
Outdated

Comment threadexamples/build_file_generation/WORKSPACE
Outdated

Comment threadgazelle/.gitignore
Outdated

Comment threadgazelle/python/python_test.go
Outdated

@aignas
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
7b0ec08 to
2ad688fCompareJanuary 5, 2023 08:03
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

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
@chrislovecnmchrislovecnm
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@alexeagle
alexeagle
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
FYI @f0rmiga is on paternity leave, back next week I believe
@chrislovecnm
chrislovecnm
commented
Jan 10, 2023

Copy link


Copy Markdown

Contributor
@alexeagle who else can review it?
@alexeagle
alexeagle
commented
Jan 11, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
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
aignas
commented
Jan 12, 2023•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
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
alexeagle
commented
Jan 12, 2023

Copy link


Copy Markdown

Contributor
Yeah I think the folder rename first will make it more manageable. Thanks!
@aignasaignas
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.
@aignasaignas
changed the title
breaking: feat(gazelle): Move the plugin to a separate workspacefeat(gazelle)!: Move the plugin to a separate workspaceJan 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
aignasforce-pushed
the
gazelle-bzlmod-5
branch
2 times, most recently
from
d1de560 to
d9bd232CompareJanuary 24, 2023 06:31
@aignas
aignas
commented
Jan 24, 2023

Copy link


Copy Markdown

CollaboratorAuthor
@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:mainJan 25, 2023
@f0rmiga
f0rmiga
commented
Jan 25, 2023

Copy link


Copy Markdown

Member
Thanks for addressing this, @aignas!
@aignas
aignas
deleted the

gazelle-bzlmod-5

branch
January 25, 2023 22:39
@renovaterenovateBot
mentioned this pull request
Jun 19, 2023
 Merged

1 task
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rickeylevrickeylevAwaiting requested review from rickeylevrickeylev is a code owner

2 more reviewers

@chrislovecnmchrislovecnmchrislovecnm approved these changes
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

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