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

⏪️ Restore previous Action outputs deployment-url and deployment-alias-url for Pages deployments #308

Closed
wants to merge 3 commits into from

Conversation

tiangolo
Copy link

⏪️ Restore previous Action outputs deployment-url and deployment-alias-url for Pages deployments

Description

When a deployment is a Pages deployment, #303 (accidentally, I think) removed the exiting GitHub Action output deployment-url and deployment-alias-url.

This was released in version 3.10 and caused #307

I found it while debugging how I use it in the FastAPI docs previews: fastapi/fastapi#12526 (comment)

Solution

This PR restores including the existing output variables even when the command is a Pages deployment (keeping the new output variables too). It also documents the new output of the Action in the action.yml schema.

Next Steps

Not sure about the process for testing, not sure if this would be included in your tests, I'll leave this up to you.

Feel free to take over this PR and add things on top, or copy the code somewhere else, etc. All's good. 😅

I think the first person to give this a first review would be @courtney-sims 🤓

@Maximo-Guk
Copy link
Member

Hi @tiangolo, thank you! Could you add a changeset, please?

For reference

@tiangolo
Copy link
Author

Thanks @Maximo-Guk, I just did.

I see you reverted the original change in #309. As there's another issue related to this change I suspect you might want to re do it all in a different way, so feel free to close this one.

@Maximo-Guk
Copy link
Member

Thanks @tiangolo , I've opened up #312 , where we're going to definitely support deployment-url and deployment-alias-url!

@Maximo-Guk Maximo-Guk closed this Oct 31, 2024
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.

2 participants