-
Notifications
You must be signed in to change notification settings - Fork 308
Tag and release on a specific commitish #32
Tag and release on a specific commitish #32
Conversation
I didn't realize there's already #19, which also fixes this issue, though that hasn't been updated in well over a month. Additionally, I think it's a more sane default to use Alternatively, one could set |
I agree with @fleskesvor that this PR should be merged over #19 . Using the current branch / sha as the default commitish is more intuitive and what I'd expect. |
Much needed feature. |
Just so that folks can copy-paste this without going through the docs: replace: uses: actions/create-release@v1 with # https://github.com/actions/create-release/pull/32
uses: fleskesvor/create-release@1a72e235c178bf2ae6c51a8ae36febc24568c5fe if you want to use this fixed version of the action. |
Thanks. 🙂 Alternatively, I'm a bit surprised (and slightly disappointed) that this PR hasn't received any feedback from the maintainers after a whole month. |
@fleskesvor, I'm afraid that there is no maintainer for this repo in GitHub's internal structure. As commented in #6 (comment), this was a side project that was released in this organisation (because GitHub didn't manage to provide an oficial solution on time for the release of the product last Nov). Fortunately, they seem to be working hard in the (non open-source) background, to extend GitHub's API so that, hopefully, better Actions can be written in the future. They are also improving the visualisation of e.g. Artifacts, which might be a valid alternative for users willing to use this action to release nightly builds. Unfortunately, it might take long until some manager finds out that giving write access to a bunch of employees without assigning any responsability is not going to work. We need someone to take the lead. That might well be an external contributor. |
@eine Oh, I see. That's unfortunate for the time being, but hopefully a better solution will come along soon. It might be difficult to get external contributors to take on that responsibility too, since a small Javascript application for a proprietary service might not be the most exciting thing to be spending time on. |
@fleskesvor, I agree. Ideally GitHub would make it explicit that this is still a beta de facto and it is going to be pretty limited feature-wise and in regard to human resources for a few months yet. That would make it much easier for us users not to feel continuously disappointed and frustrated. However, I'm neither a manager nor a marketing guy... |
@IAmHughes Any input in this? It's a tiny change, but one we need in an increasing number of repos in my organization. Even Firebase uses my fork in several of their repos. |
@@ -18,6 +18,7 @@ async function run() { | |||
const body = core.getInput('body', { required: false }); | |||
const draft = core.getInput('draft', { required: false }) === 'true'; | |||
const prerelease = core.getInput('prerelease', { required: false }) === 'true'; | |||
const commitish = core.getInput('commitish', { required: false }) || context.sha; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it behave same without the input value as it behaved before this change - i.e. just let the git API use it's default:
const commitish = core.getInput('commitish', { required: false }) || context.sha; | |
const commitish = core.getInput('commitish', { required: false }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is an option, but I don't think it's what most users of this action would expect. My suggested change doesn't inconvenience hobbyists who only commit and build on master, since context.sha
will still resolve to head of master in those cases, but it also doesn't confuse more advanced users who build on multiple branches, since it will release on the head of those actual branches, and not a totally different branch.
As I wrote on #31:
After looking into the code, I realized that the issue is that this action calls octokit.repos.createRelease() without a
target_commitish
, which makes the API default tothe repository's default branch (usually master)
. I created #32 to settarget_commitish
with what I think is a more reasonable default in the context of a CI workflow.
So yeah, I'm a bit reluctant to make that change, but still, being able to actually set a commitish is better than the current situation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fleskesvor, you are 100% correct.
Correct me if I'm wrong, context.sha
is the same as repository's github.sha
? If so, I don't see any problem with that since github.sha uses long commit-sha afaik.
unsure, but this function doesn't work for us and we are not understand why.
in enable debug for action doesn't showes more. |
@sebastian-toepfer That looks like it should work. If debug doesn't turn up any clues, you could try to print those variables in a throwaway step to see if they are what you except them to be, eg.:
|
@sebastian-toepfer @fleskesvor, I just had a similar issue. using the long commit-sha worked for me. but using the short hash ( 7 chars) results to same error: |
@fleskesvor: have replace sha with branch name it works like a charm now.
see no issue :( |
@pivotal-kaveh @sebastian-toepfer, perhaps GitHub's releases API doesn't support shorter commit-sha. |
Any plans on getting this merged? I noticed that a v1.1.0 of this Action was release a few days ago. Would be great if we can get this included for the next release. |
Would really like this pull-request to be merged! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @fleskesvor!
And thank you everyone for your patience here. Was recently made aware of this PR. Sorry for the long delay.
This is now included in If you all could use it and share if you have any issues. We can point v1 to the new tag after we gain some confidence and there are no issues. |
I was able to use this new feature successfully via v1.1.1 in a PR I opened today. I found it a little fiddly, but I think that’s actually mostly due to how many different ways there are to access commit-ish things in the context of a workflow/job execution. At one point I think I may have given it a bare branch name and seen it fail, but I’m not 100% sure. One tricky thing that we might want to document is that if one uses this action in a job that’s triggered by a Regardless of that all, this is now working for me and I’m relieved. Thank you @fleskesvor and @mscoutermarsh! |
Every pull request build is base on last known of "latest HEAD" branch with a "merge" commit. This is how continuous integration handle to verify if the pull request is able to build. You will be able to copy exact commit into your repository's url link after commit/commits path. Then see which commit was used from the branch's commits was built from. However, it's not relative to this pull request. P.S. Creating a release shouldn't happen during active pull request, only on merged, aka pushed. 😉 Cheers! |
Solves #31