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

fix: Disable python codegen support by default #6884

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

tpasternak
Copy link
Collaborator

Checklist

  • I have filed an issue about this change and discussed potential changes with the maintainers.
  • I have received the approval from the maintainers to make this change.
  • This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: <please reference the issue number or url here>

Description of this change

@tpasternak tpasternak marked this pull request as ready for review October 11, 2024 14:33
@github-actions github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Oct 11, 2024
@tpasternak
Copy link
Collaborator Author

@agluszak

@tpasternak tpasternak merged commit 7ba94e2 into bazelbuild:master Oct 11, 2024
8 checks passed
@tpasternak tpasternak deleted the disable-pycodegen-by-default branch October 11, 2024 14:37
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Oct 11, 2024
@FrankPortman
Copy link

FrankPortman commented Oct 12, 2024

I happened to see this PR when browsing this repo and this change looks potentially significant to me as I have macros/rules that generate PyInfo/files -> py_library in my codebase. Is there any documentation for this change?

@andponlin-canva
Copy link
Contributor

I happened to see this PR when browsing this repo and this change looks potentially significant to me as I have macros/rules that generate PyInfo/files -> py_library in my codebase. Is there any documentation for this change?

Hello @FrankPortman ; I added some comments here which also includes an example.

@FrankPortman
Copy link

Thanks - if my understanding is correct then this flag and tag are required to make IntelliJ properly detect and remove the "red squiggle" from codegenned files. But my basic usage of both macros and rules to code generate Python files has not had issues with IntelliJ as of now. Is this change a general safety net or only needed if you intend to output directories rather than marking individual files?

@andponlin-canva
Copy link
Contributor

andponlin-canva commented Oct 12, 2024

Yes the flag and tag will be required to make this work fully. The intention is to remove the "red squiggle" (see here) and to allow code-complete on generated code. The example project demonstrates Python code-generation not working previously for the case of a directory and the case of straight files outputs.

@andponlin-canva
Copy link
Contributor

But my basic usage of both macros and rules to code generate Python files has not had issues with IntelliJ as of now

@FrankPortman ; If I understand you correctly, you are facading your Rules' files-based (no directory) output with a py_library which is working for you in Intelli-J? This change presumes that you might be using the Rule directly and the Rule has the option of having directory-based outputs. This may be the two differences from the approach that you have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: CLion CLion plugin product: GoLand GoLand plugin product: IntelliJ IntelliJ plugin
Projects
Development

Successfully merging this pull request may close these issues.

6 participants