Skip to content

Genericize OIDC templates, forms and views #15143

Merged
di merged 7 commits intopypi:mainfrom
di:genericize-oidc
Jan 5, 2024
Merged

Genericize OIDC templates, forms and views #15143
di merged 7 commits intopypi:mainfrom
di:genericize-oidc

Conversation

@di
Copy link
Copy Markdown
Member

@di di commented Jan 5, 2024

This PR makes the OIDC templates, forms and views more "generic" and less GitHub-specific. This is a precursor to the fix for #13551 (which I have drafted). This PR doesn't introduce any functional changes.

After this, we should get #14063 merged as well, and then we can begin adding other trusted publishers.

(cc @woodruffw)

@di di requested a review from a team as a code owner January 5, 2024 00:36
Copy link
Copy Markdown
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM structurally! I'm standing these changes up locally in a moment as well.

Edit: Confirmed that it works as expected locally as well 🙂

Copy link
Copy Markdown
Member

@miketheman miketheman left a comment

Choose a reason for hiding this comment

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

Nice work, took me a bit to read through and figure this out.

A couple of questions and notes inline - nothing particularly blocking.

Comment thread warehouse/templates/manage/project/publishing.html
Comment thread warehouse/oidc/forms/__init__.py
Comment thread warehouse/manage/views/__init__.py
@di di requested a review from miketheman January 5, 2024 20:11
@di di merged commit bb5fd18 into pypi:main Jan 5, 2024
@di di deleted the genericize-oidc branch January 5, 2024 20:21
@di di mentioned this pull request Jan 11, 2024
@sentry
Copy link
Copy Markdown

sentry Bot commented Jan 18, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Connection error from GitHub user lookup API (possibly offline) manage.account.publishing View Issue

Did you find this useful? React with a 👍 or 👎

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants