heroku-review-app-actions icon indicating copy to clipboard operation
heroku-review-app-actions copied to clipboard

add option to wait for heroku build on `open` action instead of only `synchronize`

Open rawestmoreland opened this issue 2 years ago • 4 comments

I'm using this action to deploy the review app to Heroku and then deploy my frontend directory to Netlify in the same workflow.

Currently, the Netlify deploy step could be reached before Heroku is finished building the review app.

I think it would be useful to be able to wait for the Heroku build to complete on the initial open action instead of just synchronize

I can submit a PR with a solution if that's something you'd be willing to incorporate.

rawestmoreland avatar Jun 17 '22 17:06 rawestmoreland

That's a welcomed improvement. However, I'd say this should be added as a step without needing a new option, and it should only happen when it's opened or reopened.

pmbanugo avatar Jun 17 '22 17:06 pmbanugo

- name: Create Build
      if: ${{ github.event_name == 'pull_request' && github.event.action == 'synchronize' }}
      shell: bash
      run: |
        export SOURCE_GET_URL=$(echo ${{ toJSON(steps.source_endpoint.outputs.SOURCE_ENDPOINT) }} | jq -r '.get')
        export OUTPUT_STREAM_URL=$(curl -X POST https://api.heroku.com/apps/$APP_NAME/builds \
        -d '{"source_blob":{"url":"'"$SOURCE_GET_URL"'", "version": "${{ github.event.pull_request.head.sha }}"}}' \
        -H 'Accept: application/vnd.heroku+json; version=3' \
        -H "Content-Type: application/json" \
        -H "Authorization: Bearer ${{ inputs.api-key }}" | \
        jq -r '.output_stream_url')
        curl $OUTPUT_STREAM_URL

I was thinking to just refactor the if to:

if: ${{ github.event_name == 'pull_request' && (github.event.action == 'open' || github.event.action == 'synchronize') }}

But you would rather it have a new step @pmbanugo ?

rawestmoreland avatar Jun 17 '22 18:06 rawestmoreland

Yes, as a new step. I left my reason why it should be that way.

pmbanugo avatar Jun 17 '22 21:06 pmbanugo

@pmbanugo thank you for the explanation. Makes total sense. I'll update my PR 👍

rawestmoreland avatar Jun 18 '22 01:06 rawestmoreland