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

oauth: Use the new and improved Sigstore redirect page #356

Merged
merged 2 commits into from
Dec 22, 2022

Conversation

tetsuo-cpp
Copy link
Collaborator

Closes #354

</html>
"""
""" # noqa: E501
Copy link
Collaborator Author

@tetsuo-cpp tetsuo-cpp Dec 22, 2022

Choose a reason for hiding this comment

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

I've just decided to silence Ruff here as I don't think it helps readability to insert line breaks for this type of string.

@tetsuo-cpp
Copy link
Collaborator Author

Thanks @julienv3, this looks great.

@jku
Copy link
Member

jku commented Dec 22, 2022

This is (mostly?) code copied from sigstore/sigstore right? Should include some authorship info in the commit as well -- it's good for tracking copyrights but also good for the authors internet fame and fortune (not everyone cares about that but it's still nice).

Adding a "Co-authored-by:" to the commit message would trigger at least github noticing the contribution, maybe that's good enough?

@tetsuo-cpp
Copy link
Collaborator Author

This is (mostly?) code copied from sigstore/sigstore right? Should include some authorship info in the commit as well -- it's good for tracking copyrights but also good for the authors internet fame and fortune (not everyone cares about that but it's still nice).

Adding a "Co-authored-by:" to the commit message would trigger at least github noticing the contribution, maybe that's good enough?

My mistake, I completely forgot about that. I'll add the "co-authored-by" to attribute the commit to @julienv3 and add a comment in the code to explain where it's from and who wrote it. How does that sound?

@woodruffw
Copy link
Member

LGTM! Per #361 we might to eventually move this to a stand-alone template or file, since that might help with automatic syncing against cosign.

@woodruffw woodruffw merged commit 020280c into main Dec 22, 2022
@woodruffw woodruffw deleted the alex/auth-page branch December 22, 2022 15:56
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.

Use the new and improved Sigstore redirect page
4 participants