im-open / create-release

An action that creates a GitHub release
MIT License
0 stars 1 forks source link

Using the default `commitish` leads to unintentional inaccuracies. #5

Closed mike-schenk closed 2 years ago

mike-schenk commented 2 years ago

https://github.com/im-open/create-release/blob/3475a65d82549536e0b79e18661e4ab2916fc759/src/main.js#L33

This issue is in version 2.0.2 but not 2.0.1.

One example of the problem is in the releases created by https://github.com/im-customer-engagement/pega-telephony-manager/blob/main/.github/workflows/im-build-dotnet-ci.yml.

The workflow (at time of this writing) does a "normal" checkout:

      - name: Checkout Repository
        uses: actions/checkout@v2
        with:
          fetch-depth: 0

Which means it checks out (and builds) the code at context.sha, not context.payload.pull_request.head.sha.

Then the release (or prerelease) tag is created for the pull request branch's head due to the default commitish.

      - name: Create Release with published_artifacts
        id: create_release
        uses: im-open/create-release@v2.0.2
        with:
          github-token: ${{ secrets.GITHUB_TOKEN }}
          delete-existing-release: true
          tag-name: ${{ steps.version.outputs.NEXT_VERSION }}
          prerelease: ${{ env.PRERELEASE }}
          asset-path: ${{ env.DEPLOY_ZIP }}
          asset-name: ${{ env.DEPLOY_ZIP }}
          asset-content-type: application/zip

For branch builds, (pull requests that aren't yet merged), this causes all the pre-release tags to be applied in a linear history which is good, but it's inaccurate because those tags don't mark the code that was actually built. This is not a major concern for open PRs.

But for builds of main, the tags aren't in a linear history and don't match the source code that was actually built. I.e., the older release tags are not always in the history of the newer release tags, and the latest release tag never matches the tip of main. image

The existing default behavior is only correct if you specify a non-default ref: ${{ github.head_ref }} on actions/checkout to get the source to build.

I suggest one of three things:

  1. require callers to supply commitish instead of defaulting it
  2. switch back to the original default so that the default behavior of actions/checkout and this action match each other.
  3. get the default by querying the the current git working tree to find the SHA of the current HEAD instead of getting it from the workflow's context.
danielle-casella-adams commented 2 years ago

I'm conflicted about what to do here. I don't remember the exact specifics, but I changed the default for pull requests because I was getting a lot of feedback that it wasn't tagging the right thing. I think we have two camps who want different things tagged. Maybe going with no default is the way to go and people can choose what works for them.

mike-schenk commented 2 years ago

The "right" thing to tag (and to check-out and build) depends on whether it's supposed to be the build of a merged PR, or the build of a still-open PR (i.e. a build of main or a build of a branch respectively.)

Alternatively, the right thing to tag is whatever was checked-out and built. Then the question moves to checking out the right thing.

danielle-casella-adams commented 2 years ago

Maybe we could have a chat about this....

danielle-casella-adams commented 2 years ago

Just wanted to add a note here for posterity. After @mike-schenk and I talked we are going to remove the defaults for the commitish arg and rely on the user supply that info. This is addressed in #8.