Skip to content
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

add 'update_tag' option #293

Closed
wants to merge 2 commits into from
Closed

add 'update_tag' option #293

wants to merge 2 commits into from

Conversation

iTrooz
Copy link

@iTrooz iTrooz commented Dec 18, 2022

This PR adds the 'update_tag' option

Fixes #171

@sithlord48
Copy link

I need this feature for continuous releases . @softprops How can I help test this ?

@mkurz
Copy link

mkurz commented Mar 4, 2024

@softprops Can you please take a look here finally? This is a very useful feature and our project would highly benefit from it. Thank you very much!

@chenrui333 chenrui333 added the bug Something isn't working label Jun 19, 2024
@chenrui333
Copy link
Collaborator

@iTrooz can you fix the merge conflicts? Thanks!

@chenrui333
Copy link
Collaborator

@iTrooz ping

@iTrooz
Copy link
Author

iTrooz commented Jul 19, 2024

Hey, unless the maintainer shows interest in merging this, I'm not interested in updating it, sorry

@iTrooz
Copy link
Author

iTrooz commented Jul 19, 2024

Oops, I just realized that you are a maintainer of this repository. I will update this PR soon

@chenrui333
Copy link
Collaborator

yeah, please :)

@iTrooz
Copy link
Author

iTrooz commented Jul 22, 2024

Why did I think making a refactor on a feature PR was a good idea..

Anyway, done !

Note that I have no idea how github orders release, using update_tag may not bring the release to the top. But it will avoid a spam of notifications to users subscribed :)

@iTrooz
Copy link
Author

iTrooz commented Jul 22, 2024

@chenrui333 Ready for review/merge

I already tested this PR, but if anyone wants to double-check, please do so


// give github the time to draft the release before updating it
// Else, I think we would have a race condition with github to update the release
await sleep(2000);
Copy link
Owner

@softprops softprops Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is something inherently unreliable and inefficient about relying on time as a synchronization primitive

It’s possible that sleeping 2 seconds isn’t long enough resulting in a noneffective approach and it is also possible that is is way to long resulting in wasted gh action time.

Is it possible to restructure this without so that it’s not time dependent?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fully understand why sleeping is not a perfect solution.
That said, this is certainly the easiest option to implement.

Another solution that could work would be to make a request to the github API every second to check if the draft was done yet. Would you prefer this ?

@@ -194,6 +194,7 @@ The following are optional as `step.with` keys
| `generate_release_notes` | Boolean | Whether to automatically generate the name and body for this release. If name is specified, the specified name will be used; otherwise, a name will be automatically generated. If body is specified, the body will be pre-pended to the automatically generated notes. See the [GitHub docs for this feature](https://docs.github.com/en/repositories/releasing-projects-on-github/automatically-generated-release-notes) for more information |
| `append_body` | Boolean | Append to existing body instead of overwriting it |
| `make_latest` | String | Specifies whether this release should be set as the latest release for the repository. Drafts and prereleases cannot be set as latest. Can be `true`, `false`, or `legacy`. Uses GitHub api defaults if not provided |
| `update_tag` | Boolean | Update the tag of the release to the current commit. This will also update the release time. Default is false |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some reservations about this. Action gh release is an action for managing GitHub releases, the product feature

the concern is that with these changes the scope is increasing to also include managing git state something that feels possible with git itself external to this action.

Would it be possible to do that as a separate workflow step or possibly trigger that then triggers the workflow that manages gh release data?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would definitively be possible to do this in another step, and I understand why this may be preferable to extending the scope of this action.

So at this point, it may be better to just use the solution described here : #171 (comment)

@iTrooz
Copy link
Author

iTrooz commented Jul 30, 2024

Closing in favor of #171 (comment) (See also #293 (comment))

@iTrooz iTrooz closed this Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release time does not update
5 participants