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

Remove the log_url to stop target_url being ignored. #26

Merged
merged 1 commit into from
May 21, 2022

Conversation

jimCresswell
Copy link
Contributor

Closes #25

@tony
Copy link

tony commented Sep 30, 2021

Thank you!

@jimCresswell Any difference between yours and @ypresto's at #18? It this something you want to join forces on?

@@ -30,7 +29,6 @@ async function run() {
...context.repo,
deployment_id: parseInt(deploymentId),
state,
log_url: logUrl,
target_url: url,
Copy link

Choose a reason for hiding this comment

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

I guess we may as well rename target_url to log_url, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @tony. I think the fundamental problem is that I want a URL for a deployed app, which should live in environment_url, but that field isn't included in deployment_status events in Github (at least not in a way that is visible inside Github workflows). log_url should replace target_url as in @ypresto's PR, but doing so means the data in target_url isn't passed to the deployment_status event. So honestly I don't think my PR should go in, the real fix is probably to find out why environment_url isn't being passed around, and fix it or propose a change, and in the meantime it's necessary to hack what should be the environment URL into the free-form description field.

Copy link

Choose a reason for hiding this comment

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

@jimCresswell Thanks for the response. I don't fully understand your use case, likely due to my own naiveite of Deployment API at the moment.

So if you do develop a one-size-fits-all case, that would be good (if you have time). If not no problem. We'll also see if @ypresto has anything to say

@naquiroz
Copy link

naquiroz commented Dec 3, 2021

Hi, can this be merged?

@chrnorm
Copy link
Owner

chrnorm commented May 21, 2022

Thankyou @jimCresswell and my apologies for the absence here, it's been a couple of years since I've used the Deployments API with GitHub Actions and I've let this repo go unmaintained. I will be pushing a new version in line with deployment-action@v2 which will expose more parameters as input variables.

@chrnorm chrnorm merged commit d2d29a3 into chrnorm:master May 21, 2022
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.

target_url doesn't make it through to deployment_status events
4 participants