Added commitish as input

[AndreBlumenthal]

In this PR, we would like to add the target_commitish property for creating a release. This is also contained in the original repository here. The problem we want to solve with it, is that the action is always tagging the default branch (main in most cases) but not the branch the action is running on. By providing the target_committish property, you can steer where to put the newly created tag.

Edit: We made the test run through but they need another look. Right now the tests are not bullet proof. The jest mocks are created with the assumption that core.getInput is always called in the same order. This was the case in the original version but it is not anymore.

[AndreBlumenthal]

it is debatable whether or not we want to default to context.sha or just leave it undefined if not provided by the user.

When we leave it undefined we don't impact current usages of this action

[anzz1]

Most definitely should default to context.sha as that is the only way that makes sense. The current implementation (without this PR) where the created release doesn't match the action it's ran from is absolutely unintuitive.

So for example, if you run a build action to compile binaries and release them from other branch, or from an earlier commit, the release itself would be tagged to the master branch and latest commit while the binaries would be from a different one and thus the release information and the source code wouldn't match the compiled binaries. I highly doubt that there is any user out there who intends to do this.

For the less popular use cases where the action is not ran against a commit, like from cron jobs, commit.sha == undefined anyway so that wouldn't change.

I think it's quite obviously intuitive that when you run an action from whatever branch or commit, that the release should match that commit unless specified otherwise. That's exactly how you did it, and that's exactly how it should work. I don't think it's even debatable, as I cannot imagine a counter-argument against the logic how you currently have it.

I'd say the correct way of making sure even the most weirdest of configurations wouldn't get affected whatsoever, is not to use the old flawed logic of undefined but rather bump the version tag from v2 to v3.

[anzz1]

So a TL;DR; version for the maintainer:
Change nothing, keep this PR exactly as it is now, and when you merge this just bump the version tag to v3 afterwards, that's it.

[anzz1]

Please merge.

[AndreBlumenthal]

I guess this will not happen. I close this now

[anzz1]

I guess this will not happen. I close this now

I hope you reconsider, since this is an absolutely essential fix.

I too do see that this action seems to be largely abandoned, but it still works perfectly fine with your changes. I've forked this change https://github.com/Jimdo/action-create-release and have personally tested it by using it for months now over various repos and hundreds of deployments, and it has worked flawlessly 100% of the time^[*].

It's a small change after all and doesn't break any existing functionality. It's currently impossible to use multiple branches, create mixed releases/prereleases, or create releases from earlier commits than the latest master without this. Nothing will change for existing users who so far could only use a single master branch and create releases from the latest commit, as that will keep working just as it did. This is not a breaking change, and there is absolutely no reason to not merge this. And the version tag would be bumped from v2 to v3 anyway after the merge.

Using the fork obviously works fine too, but ultimately it'd be better to get it merged to this main project as this action is widely used and referenced to. It'd save the headache of troubleshooting and having to dig for this working fork. Also by closing the PR, it gets buried to the closed PR's and makes it harder for other people find the fix, and also if the maintainer ever comes back to check on this project, they'll probably miss the problem altogether.

It doesn't hurt to have this issue open till the end of time if necessary, right?

_{[*]: With the exception of GitHub outages, but obviously nothing works during outages due to no fault of the script itself.}

[AndreBlumenthal]

I guess this will not happen. I close this now

I hope you reconsider, since this is an absolutely essential fix.

I too do see that this action seems to be largely abandoned, but it still works perfectly fine with your changes. I've forked this change https://github.com/Jimdo/action-create-release and have personally tested it by using it for months now over various repos and hundreds of deployments, and it has worked flawlessly 100% of the time[*].

It's a small change after all and doesn't break any existing functionality. It's currently impossible to use multiple branches, create mixed releases/prereleases, or create releases from earlier commits than the latest master without this. Nothing will change for existing users who so far could only use a single master branch and create releases from the latest commit, as that will keep working just as it did. This is not a breaking change, and there is absolutely no reason to not merge this. And the version tag would be bumped from v2 to v3 anyway after the merge.

Using the fork obviously works fine too, but ultimately it'd be better to get it merged to this main project as this action is widely used and referenced to. It'd save the headache of troubleshooting and having to dig for this working fork. Also by closing the PR, it gets buried to the closed PR's and makes it harder for other people find the fix, and also if the maintainer ever comes back to check on this project, they'll probably miss the problem altogether.

It doesn't hurt to have this issue open till the end of time if necessary, right?

[*]: With the exception of GitHub outages, but obviously nothing works during outages due to no fault of the script itself.

I really appreciate your commitment to this. Also a huge thanks to your comment about the commitish. The PR was open now for almost half a year and so far there is not a single reaction at all from the maintainers. This is why I don't expect that anything will happen on that matter.

[anzz1]

I really appreciate your commitment to this. Also a huge thanks to your comment about the commitish. The PR was open now for almost half a year and so far there is not a single reaction at all from the maintainers. This is why I don't expect that anything will happen on that matter.

I do appreciate when stuff works properly, so thank you. @zendesk is not personal account but a company one, and it's quite possible this PR has just slipped under the radar. I'd say give it a bit more time, but ultimately it's up to you.

edit: If this repo allows it, you could try requesting a review under the "Reviewers" label so someone actually gets notified personally. There are almost 1K repositories under this account so probably most of the notifications under the company account slip by unseen.