Skip to content

Conversation

@dbhatman
Copy link
Contributor

@dbhatman dbhatman commented Oct 4, 2022

This replaces the need for #1016

PiperOrigin-RevId: 478889967


This change is Reviewable

@lalten
Copy link
Contributor

lalten commented Oct 8, 2022

Thanks for taking a stab at this @dbhatman. This does indeed fix the issue with Pylance described in #1016, but it adds another problem: Go-to-definition (in VS Code) will now always jump to __init__.py instead of the actual definition. The solution in #1016 does not show this behavior, it jumps right into the actual source file.

@dbhatman
Copy link
Contributor Author

Thanks for taking a stab at this @dbhatman. This does indeed fix the issue with Pylance described in #1016, but it adds another problem: Go-to-definition (in VS Code) will now always jump to __init__.py instead of the actual definition. The solution in #1016 does not show this behavior, it jumps right into the actual source file.

Thanks for following up @lalten

The behavior (ie requires the second click of go-to) you are seeing is pretty typical in projects following google style guides because of this style rule: https://google.github.io/styleguide/pyguide.html#22-imports. As part of cleaning up some things on our side so we can more closely align with the style guide, I am going to suggest we go forward with this but I'll keep this issue in mind and see if I can come up with an alternate solution that still complies.

@lalten
Copy link
Contributor

lalten commented Oct 11, 2022

Lgtm then. If you're in the process of cleaning up it would be great to give the pylintrc a refresh as that's currently broken (references deleted plugins and such).

@dbhatman
Copy link
Contributor Author

Ah was just looking at the pylintrc and scratching my head at the plugins!

(and cleaning that up broke this PR 😆)

This replaces the need for google#1016

PiperOrigin-RevId: 477311168
@dbhatman dbhatman reopened this Oct 11, 2022
@dbhatman dbhatman merged commit 141d990 into google:master Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants