Skip to content

Conversation

@miketheman
Copy link
Member

warehouse/manage/views/__init__.py is already a complex module, with
over 3k LOC. Split out any OIDC code (conveniently in a single class) to
a new module.
This should allow for more focused edits when there are changes.

Signed-off-by: Mike Fiedler miketheman@gmail.com

`warehouse/manage/views/__init__.py` is already a complex module, with
over 3k LOC. Split out any OIDC code (conveniently in a single class) to
a new module.
This should allow for more focused edits when there are changes.

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman miketheman requested a review from a team as a code owner October 7, 2025 18:31
@miketheman miketheman added developer experience Anything that improves the experience for Warehouse devs trusted-publishing labels Oct 7, 2025
@di
Copy link
Member

di commented Oct 7, 2025

Makes sense to me to pull these out. I would expect us to use the same organizational pattern we use elsewhere though, where we create a views directory instead, which would contain __init__.py, publishers.py, etc. (I would also expect to pull the related tests out into a separate file since these are generally 1:1).

@miketheman
Copy link
Member Author

I agree - however these are manage/ views, and are already collected under that namespace, following their routes. We have oidc/views.py for the non-management views.

I'll move the tests out in another commit.

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@ewdurbin
Copy link
Member

ewdurbin commented Oct 7, 2025

I'll move the tests out in another commit.

another commit or another PR?

@miketheman
Copy link
Member Author

I'll move the tests out in another commit.

another commit or another PR?

commit - 7069bbc

@ewdurbin
Copy link
Member

ewdurbin commented Oct 7, 2025

Makes sense to me to pull these out. I would expect us to use the same organizational pattern we use elsewhere though, where we create a views directory instead, which would contain __init__.py, publishers.py, etc. (I would also expect to pull the related tests out into a separate file since these are generally 1:1).

Yeah, If we're going to further churn this, git mv warehouse/manage/views.py warehouse/manage/views/__init__.py along with the others seems like a sane thing to just rip all the bandaids at once.

@di
Copy link
Member

di commented Oct 7, 2025

Yeah, If we're going to further churn this, git mv warehouse/manage/views.py warehouse/manage/views/init.py along with the others seems like a sane thing to just rip all the bandaids at once.

I'm doing a double take because I thought warehouse/manage/views.py existed but it doesn't, nevermind me!

@miketheman
Copy link
Member Author

Confused - git mv warehouse/manage/views.py - I did that way back in 546f16c (#13187) ?

@ewdurbin
Copy link
Member

ewdurbin commented Oct 7, 2025

Confused - git mv warehouse/manage/views.py - I did that way back in 546f16c (#13187) ?

always look at the code, not the chatter :) my bad.

@miketheman miketheman merged commit 17aafa5 into pypi:main Oct 7, 2025
21 checks passed
@miketheman miketheman deleted the miketheman/refactor-oidc-views branch October 7, 2025 19:13
@di di mentioned this pull request Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer experience Anything that improves the experience for Warehouse devs trusted-publishing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants