Skip to content

Fix deploy triggered by workflow_dispatch #8

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

Merged
merged 3 commits into from
Oct 24, 2021

Conversation

mkarnicki
Copy link
Contributor

@mkarnicki mkarnicki commented Jun 17, 2021

After console logging github.context I found that sha lives on github.context not github.context.payload so if head_commit is not defined, we use github.context.sha instead. Thanks for considering this fix.

Fixes #7

mkarnicki and others added 2 commits June 17, 2021 23:34
This would probably be equivalent, but I've never written a GitHub Action so I took the safe route here.

`const commitId = isPullRequest ? payload.pull_request.head.sha : github.sha;`

Fixes webfactory#7
@mpdude
Copy link
Member

mpdude commented Jul 5, 2021

Hey @mkarnicki,

sorry it took me so long to get back to this.

The change looks reasonable, although I must admit I always have a hard time figuring out what information to pick from the event context.

Could you do me a favor and run the change as suggested on your own repo, using the workflow_dispatch event?

You should be able to do so by specifying uses: mkarnicki/create-aws-codedeploy-deployment@patch-1 in your workflow file.

Does it correctly pick up the commit SHA, and is the branch name correct as well?

@mkarnicki
Copy link
Contributor Author

I must admit I always have a hard time figuring out what information to pick from the event context.

I feel you :D!

Certainly, I'll do that and report back, thanks for your time!

@mkarnicki
Copy link
Contributor Author

mkarnicki commented Jul 6, 2021

Hrm. Doesn't seem to have done the trick. The output is:

🎋 On branch 'main', head commit undefined

I printed out the following:

          echo ${{ github.sha }}
          echo ${{ github.ref }}

with the output being:

27c7c8c265ddbbc828f3c88f9b1063011ba83de2
refs/heads/main

So github.sha is correct, but it's value wasn't used 🤔 Maybe payload.head_commit is defined, but payload.head_commit.id is not 🤔 ? Edit: no, that wouldn't make sense at all 🤔

@mkarnicki
Copy link
Contributor Author

I was able to figure it out by logging github.context while running the workflow. The sha lives on github.context, not within the payload. I correctly got this message 🎋 On branch 'main', head commit 40841[...] and it is deploying as expected. I'll test it with a feature (non-main) branch soon and keep you posted :)

@mkarnicki mkarnicki force-pushed the patch-1 branch 3 times, most recently from 4a61165 to 536b870 Compare July 8, 2021 09:24
@mkarnicki
Copy link
Contributor Author

@mpdude I confirm the fix works as expected for both main and feature branches :)

@mkarnicki
Copy link
Contributor Author

Any chance go get it merged :)? We've been using it successfully for a client for a while now :) (both on push and on workflow dispatch).

@mkarnicki
Copy link
Contributor Author

We've been using this with success for a while, but our workflows still point at my patched version. Any chance this can get merged, please :)? Thank you for your consideration and have a great day!

@mkarnicki
Copy link
Contributor Author

Last polite to call for including this fix. If for some reason this does not match your requirements, we will keep using a fork.

Thanks and have a great day!

@mpdude mpdude merged commit cb5e208 into webfactory:master Oct 24, 2021
@mpdude
Copy link
Member

mpdude commented Oct 24, 2021

Thank you @mkarnicki for your contribution, and sorry for repeatedly losing sight of this!

@mpdude
Copy link
Member

mpdude commented Oct 24, 2021

Released as v0.3.0.

@mkarnicki
Copy link
Contributor Author

No problem and thank you for accepting the PR! Have a great day!

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.

Cannot read property 'id' of undefined - in on: workflow_dispatch
2 participants