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 magic link summary #270

Merged
merged 2 commits into from
Sep 28, 2024

Conversation

facutuesca
Copy link
Contributor

This PR fixes a few issues in the magic link nudge logic:

  1. Part of the nudge is printed in the summary even if Trusted Publishing is used
  2. The summary message includes the ::warning... tag, which only works for the logs, not for the summary
  3. The summary message does not include the magic links

See this job for an example of all 3 issues. The workflow uses Trusted Publishing, but the nudge is still included in the summary, with the ::warning tag and no magic links.

This PR fixes the issues by:

  • Not adding the nudge to the job summary when using Trusted Publishing
  • Constructing the summary without the ::warning tag
  • Adding the magic links to the summary

cc @woodruffw @webknjaz

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great!

@webknjaz webknjaz added the enhancement New feature or request label Sep 28, 2024
@webknjaz webknjaz merged commit 85a5a80 into pypa:unstable/v1 Sep 28, 2024
2 checks passed
@webknjaz
Copy link
Member

See this job

By the way, you really shouldn't give the transitive build deps access to OIDC...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants