-
-
Notifications
You must be signed in to change notification settings - Fork 328
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
Announce gitoxide
releases via discussion comments
#1692
Conversation
Some users and developers, such as downstream package maintainers, would benefit from a way to subscribe on GitHub to notifications for releases associated with the `gitoxide` crate, i.e. the releases done via `release.yml`, but not of the many `gix-` crates that they would also be notified about if they watched releases via the GitHub watch feature. The idea here is to create a way for people to easily be notified by subscribing to a specific locked discussion thread that a GitHub Actions job posts after publishing a `gitoxide` release. I am testing this with #6, but that is just for testing. If this is used, the actual announcements "discussion" thread would be separate, and on GitoxideLabs/gitoxide (not on a fork).
For testing, this checks the discussion title only when the release is unpublished. Just for now, it does not also do the check in the previously covered case where the release is named to indicate that it exists to test releasing rather than being a "real" release. Really, we should test both, and we should probably check the name first, since when both apply, the name indicating that it is not even an actual release is the more important reason to avoid announcing it by commenting in a discussion for real release announcements. But the name check is temporarily commented out, in order to more easily test the draft status check. This checks whether the release is a draft by actually accessing the release, rather than repeating the check from the final step in the preceding `publish-release` job for whether it *should* be published, and also rather than storing and retrieving information about whether that job did publish it. The reason to actually check the release is that the draft status can be changed manually in either direction at any time: - Changing it between the jobs is plausible if one notices that something is unexpectedly broken (or unexpectedly okay). - But the more important reason is that we have to check each time, rather than using any state from the previous jobs we depend on, if we are to work as expected when the `announce-release` job itself is re-run (which makes sense to do, for example, if one has fixed a problem and manually marked a release non-draft). This change also refactors the steps, combining the two last steps in the job. The combination of the new `gh` invocation to check the draft status, and the consolation of those two steps, makes it so `GITHUB_TOKEN` is used in all steps and `REPOSITORY` is used in multiple steps. So this moves them from per-step `env` to job-level `env`. That leaves `DISCUSSION_URL` as the only step-level `env` key. Since it's not sensitive, nor likely to be used accidentally anywhere it shouldn't, it is also moved to the job-level `env` so it's easier to see what the job depends on.
Draft releases are viewable to users with write permissions on the repository. Accordingly, for a GitHub tokens for a GitHub Actions job to be able to view draft releases, it needs `contents: write`, even if (as here) it is neither changing any state associated with the release nor accessing other repository content in any way. (It does read and write the discussion, but `content` doesn't cover that.) Otherwise, attempting to view the release would have the same effect whether the release exists but is a draft, or does not exist at all. Even in testing, we do not want the job to go ahead and announce a nonexistent release, such as one that has been deleted (including deleted after the job has run but before it is re-run), into a discussion. So it can be useful to distingiush those cases.
Now that the case where the release's name does not cause it to be treated as a test release, but where the release is not announced to a non-test discussion thread because it was found not to be published (or was published but was marked back to draft), has been manually tested, this commit uncomments the name-check case. That is to say that this makes it so a release named as being for testing, rather than being a "real" release, will trigger the discussion title check. That way, releases that exist just to test releasing are not inadvertently announced to discussion threads intended for notifying about actual releases. This check is done first, since it is the primary case for not announcing a release that is otherwise eligible to be announced, and because the log message for it makes more sense when both reasons to do the title check apply. At least for now, when the release name indicates the release is for testing, the draft status of the release is not retrieved or examined. (However, as a possible future direction, it might be worthwhile to check all three things -- release name, release draft status, and whether the discussion is titled indicating it is for testing -- and report them in the GitHub Actions workflow/job log, for all combinations.) This commit also adds a comment on the `announce-release` job saying what it's for, as we have on the other `*-release` jobs.
gitoxide
releases to a discussion thread for that purposegitoxide
releases by commenting in a "discussion"
gitoxide
releases by commenting in a "discussion"gitoxide
releases via discussion comments
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.
This is fantastic, thanks so much! I can't wait to see this in action in a couple of days!
The job is quite advanced as well and I am always impressed by the skill that enables it. Thanks to you I can't tell myself anymore that "this can't be done in shell" :D.
For completeness, here is the variable configuration to link to the release discussion, hoping that this will work.
I could have used an environment variable as well but think that these are accessed through a different prefix, so this should be correct.
Thanks! Is that to say that this PR is ready to be merged?
Looks good. Thanks!
Yes, in the It's possible to write a workflow that accepts a variable as being either in |
Apologies, I forgot to click the button 🤦♂️.
Definitely no need to complicate things, I am just happy I picked the right place for the variable configuration, and will be looking forward. I also have added a paragraph about that to the upcoming report. |
I'm happy to see that the new job worked, and #1693 (comment) was posted automatically for the v0.39.0 release! When checking that, I noticed that it looked like the release was publicly viewable even before all files were attached. I certainly have no objection to this, especially it was intentional. But as far as I know, it should only happen if the release is manually marked non-draft sometime between when the If nothing like that happened, then I can look into it. |
It worked like a charm, thanks again! Indeed, I didn't interact with the release page at all, for all I know, these have always been made public in the moment To fix this, it might be easiest to set the release back to draft mode when the release-artifact builds are started. |
I don't recommend automatically setting releases back to draft, because this will be confusing to users who see a notification (in their GitHub feed or otherwise) of the release, then find that the release is gone. Another concern is that I worry publishing and then dispublishing releases may be incompatible with some tools that might be in use to automate things in response to new releases. If the release is seen to exist, a tool may not handle its unexpected disappearance. Even if all tools are free of race conditions, they will not necessarily handle the case well by waiting or scheduling themselves to re-run; they may just report a failure and do nothing more until a user decides how to handle it. (As far as I know, users without write access to the Instead, could If I understand correctly, its Since
If I am right that these are the most useful rules to choose from, they could be made specifiable through command-line arguments. The justification for (3) is that it's when a crate provides binaries that additional work often must be done to create prebuilt executables and attach them to the release. I think it be fine even to keep the default behavior as (1), so long as an option is available to select (3), which could be used when using The gitoxide crates with binary executables are:
Other things that appear in the output of Edit: The other dimension of this is the question of what exactly the effect currently is of the command in this step of gitoxide/.github/workflows/release.yml Lines 69 to 72 in b34d14e
Last time I revised that part, I think I believed either that this was creating a release that did not yet exist (draft or otherwise), or that it would have no effect if a release for that tag and with that title already existed. But when the intended release already exists, I'm not sure attempting to create it again really is idempotent. Testing suggests otherwise. Commands of that form ask questions if they think they are being run interactively. To produce a situation similar to what happens on CI, I've been running : | gh release create v1 --title=v1 --draft in a local clone of this repository that I use for testing. To produce a similar situation, I created a release by other means first (using the web-based interface). The release is titled Then, when I ran the above (The draft release also has an earlier date. I think that's from the commit. I'm not clear on why that happens.) This all raises the question: does the I'm sure this can be done much more elegantly, but this should list them: gh -R GitoxideLabs/gitoxide release list --json createdAt,isDraft,name,publishedAt,tagName --jq '.[] | select(.isDraft == true) | [.createdAt,.name, .publishedAt, .tagName] | @tsv' Subsequent accesses to the release are by its name or tag name, and those will find the one that was created previously. For example, when I run |
Thank you for the exhaustive analysis! I very much like the idea of adjusting Running the command-line provided here yields only a single draft release (
So it does look like these will be piling up. As the binaries that are produced here are mostly for debugging/developer use, I always found them secondary to the release itself. If the notification in the release thread would only be produced once all release-builds are done, then that would also serve the purpose I think. As long as |
I'll open an issue on cargo-smart-release so this idea is not forgotten, and so I can share progress on figuring out how to do it (though I have no such progress yet).
Thanks, I had gotten the wrong idea that
Since there are not many still-draft releases piled up so far, you could probably delete them manually if you like, such as through the web-based interface. Deleting them from using the
What binary will #1238 use? I had envisioned that it would use
In part, yes. There are two caveats that I can think of:
I just temporarily tried having my separate "machine account" @NeopreneSock watch this repository in two ways, where the number of watchers shown started out as 56:
Based on this, I think at least 56 accounts are receiving notifications for all gitoxide tag/crate releases. (It is probably exactly 56 accounts, but it could be more if some people use a custom watch but do not uncheck "Releases".)
Sorry, I should not have said "notification" in #1692 (comment). The release notification discussion thread created in this PR does solve the problem of allowing people to be notified when binary releases are ready. People who wish to watch this repository can do a custom watch with all boxes checked except the box for Releases, in which case they will not receive any misleading notifications related to releases. However, what I meant to express in #1692 (comment) was:
That particular concern is still specific to marking published releases back to being drafts, so it's not something we have to worry about if we're not doing that. More broadly, however, people are likely to have the new existence of a releases surfaced to them under various conditions other than subscribing for notifications, anytime they have an algorithmically curated feed, and such a feed is built into the GitHub web interface. By default, it shows events based on guessed interest, stars, and events of interest to users one follows. This can include releases (unless one customizes one's feed to not see any projects' releases). So some people are probably finding out that way about releases even before they are populated. This is part of what I had been thinking about. I consider this a sufficient reason not to routinely mark published releases back to draft. But so long as we're not doing that, I think it's not much of a problem. Anyway, the two changes described above for how to make partially populated releases less confusing would help users who find releases through feed events, too, so long as they are not routinely dispublished. Whether those changes are worthwhile, I am not sure. A more significant reason to prefer to start |
It really is no problem to do that by hand, and I just removed the two drafts using the web-UI.
A good point, I hadn't thought about it. As the binary it uses is more of an implementation detail, it could start out using
Thanks for elaborating, I think we are on the same page. Implementing this in Thanks again for not letting go of this problem until it is resolved, it feels close and very much achievable now. |
Actually, it seems to me that making GitHub releases for all versioned crates that are developed in this repository is a good thing, and I very much recommend continuing to do so. Users who have an unfiltered watch on the repository presumably want to receive, or are at least okay with receiving, those notifications. I furthermore expect that to be more the case now that there is an alternative way (introduced in this PR) to easily subscribe to just notifications about releases of the Much more important, there are a number of benefits I think come from publishing GitHub releases for all
So, that is my case for continuing the current practice of publishing GitHub Releases for the gitoxide library crates ( |
Thank you for making the case so clearly! Let's keep GitHub releases then :)! For posterity, I also state that I had no intention to stop making them either, as with the automated notification for |
Ah, okay. I may have misunderstood the significance of that idea in what you were saying! Thank you for bearing with all that text anyway. And maybe it will be useful in the future if, ironically, it ever comes to be false: it could happen that future circumstances might outweigh those benefits, so perhaps having them articulated there will make it easier to tell whether that has happened. |
Fixes #1311
This repository produces releases associated with all of the gitoxide-related crates:
gitoxide
, but also the manygix-*
crates (andgitoxide-core
). The narrowest readily available way to use GitHub to subscribe for notifications about releases has so far been to watch all releases (Watch → Custom, "Releases"). But this is not specific to releases associated with any particular crate.It is possible to set up something to filter tag names, but that is inconvenient if one is not already doing similar automation. It would not integrate with ordinary uses of GitHub's web-based or emailed notifications. Furthermore, the
gitoxide
crate differs from the others in some important ways. One way is how, because it ships binaries, it is the crate most often packaged in downstream distributions. I believe this the main ongoing situation described in #1311.Secondarily, but also related to ameliorating #1311, it seems to me that having an easy way to subscribe specifically to announcements for GitHub releases associated with the
gitoxide
crate (i.e.,v*
tag releases) would also make it somewhat easier for users unfamiliar with the project's structure to identify which releases are "top-level." In particular, although the main binary for gitoxide shall one day beein
, currentlygix
probably has the most functionality that is regularly used, and there is also a crate namedgix
, which is a library crate providing facilities such asRepository
, and which is not the crate that carries thegix
binary.Fortunately, it is possible to allow users to subscribe to such notifications through GitHub, by having them automatically posted in a discussion created for that purpose. This pull request adds an
announce-release
job to the release workflow, that runs after thepublish-release
job and creates a discussion comment saying that the release has been created and linking to the release page for it. This is usable because:discussions: write
permissions.discussions: write
permissions are automatically sufficient to post new comments even in locked threads.It seems to me that such an automatically commented discussion thread is likely to help some downstream package maintainers, and to be of interest to some other users.
I tested this in a discussion in my fork, EliahKagan#6, which can be examined to see what the comments look like.
As for the hyperlinks in them, for releases that are actually published, the links go to the releases page. That same URL for an unpublished release goes to the tag page. This can be confirmed by comparing the URLs:
If this PR is to be merged, then there are three changes to this upstream repository that should also be made:
A new discussion should be created and given an appropriate title and brief body explaining its purpose.
The title should not start like
test
,[test
, or(test
or any case variants: when a release seems like it is for testing releasing rather than being a "real" release, the job will refuse to post a comment about it unless the discussion is titled that way. Since this would be the "production" discussion thread, we want that refusal.The new discussion should be locked. (It would be fine to allow reactions to be posted, though, since those do not generate notifications. One chooses whether to allow them when locking the discussion.)
A new GitHub Actions variable named
RELEASE_ANNOUNCEMENTS_URL
should be created, and given the full URL of the discussion as its value.I would be able to do (1) myself and would be happy to do so, but I would not be able to do (2) or (3).