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

closing-remarks: options to exclude pre-release tags (feature request) #7206

Closed
wookayin opened this issue Jan 8, 2024 · 7 comments · Fixed by #7243
Closed

closing-remarks: options to exclude pre-release tags (feature request) #7206

wookayin opened this issue Jan 8, 2024 · 7 comments · Fixed by #7243

Comments

@wookayin
Copy link

wookayin commented Jan 8, 2024

Description

The feature closing-remarks would show in which tag the PR (commit) first appeared:

This pull request first appeared in nightly

However, when the project publishes nightly or some sort of preview (unreleased) tag, it will show nightly as the first tag. This is usually not an informative information; instead, users would want to see the first "release" version, excluding all the pre-release tags.

Suggestions

  • Exclude pre-release tags by default (or upon some configuration)
  • If none of the stable releases (tags) include the commit, then show the first pre-release tag
  • It would be even better if users can configure explicit patterns to ignore tags

Example URLs

PR: https://togithub.com/neovim/neovim/pull/22060

Merged commit: https://togithub.com/neovim/neovim/commit/27b81af19c498892f4b0444ad29b7be842f8e7b8

Shows "This pull request first appeared in nightly"; should be "first appeared in v0.9.0." (the commit is included in v0.9.0, ..., v0.9.5, stable, and nightly).

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Jan 8, 2024
@fregante
Copy link
Member

fregante commented Jan 8, 2024

That's a misuse of tags that causes conflicts at every push/pull, see note in https://stackoverflow.com/a/19300065

nightly should be a branch because that's that they're for.

@fregante
Copy link
Member

fregante commented Jan 8, 2024

It would be even better if users can configure explicit patterns to ignore tags

We can't add options to Refined GitHub, see:

https://github.com/refined-github/refined-github/wiki/%22Can-you-add-this-feature%3F%22#3-it-doesnt-require-options

@wookayin
Copy link
Author

wookayin commented Jan 8, 2024

I see, I understand that options are too much burden.

But I respectfully disagree that nightly should be a branch not a tag, because "nightly releases" are "releases". Who cares a nightly tag causes a conflict? Clobbering existing tag can be easily dealth with git fetch (no tag fetching) or git fetch --tags --force should be of practically no problem.

UPDATE: Even if one uses a different, unique tag name for each nightly release; say nightly-20240101, closing-remarks will still show "This PR first appeared in nightly-20240101" which is still not useful information, as the version is a "pre-release" not a stable "release".

@fregante
Copy link
Member

The main issue is that this feature answers the question:

  • has this PR been released yet?

Pre-release is literally a release. If 1.0.1-beta includes the PR, should we said "This PR hasn't been released yet"? Because that sounds like a lie/wrong.

I'd potentially classify nightly releases as releases, but maybe we can make an exception because you probably still want to release manually later.

Perhaps we can:

  • exclude tags that include "nightly" in their name
  • exclude tags that don't include numbers

@wookayin
Copy link
Author

wookayin commented Jan 10, 2024

People would be mostly looking for:

  • Has this PR been released yet -- in a stable release?

while some might be also interested whether the PR has appeared in any pre-release versions (e.g. snapshot versions or release candidates).

Checking the tag name/pattern such as nightly or digits would work (#7208) but this might too specific given that you don't want to allow options; there might be many corner cases. However, in Github releases there is a very clear binary signal telling: Pre-release v.s. normal/stable/full release. (See #2239, #2905, Manage releases - item 10, and Releases REST API - prerelease boolean field) So I think excluding pre-release would make a sense, and this would have the same principle as #2905.

So my proposal is: to show either the first non-pre-release, the first pre-release as a fallback (if there is no stable release out), or both.

@fregante
Copy link
Member

A couple of issues with that:

  • stable or not, if it's tagged, generally you can install it, or expect it to appear soon in the non-beta version
  • the feature targets tags, which are widely used, not github releases, which are the only ones with the "pre-release" flag

I don’t think I want to overcomplicate this with logic that may vary across repos. The feature answers that question alone and it can't afford to make guesses around which tag the user prefers to see.

Considering that probably there isn't even an API for this (i.e. get the tags that contain this commit), we can't even begin to consider releases.

@wookayin
Copy link
Author

wookayin commented Jan 10, 2024

Thanks for your help and support! Yes, I do see that this is blocked by a technical limitation that the current Github REST API lacks something like git name-rev.

A side note is that, if this feature worked as you intended (as in #7208) then it should have shown "v0.9.0" instead of "nightly". So it's the same issue as #7208 -- nightly is shown because it is the alphabetically (not chronologically) the first tag that contains the commit, coming earlier than v..., due to the current way how the tag information is fetched (branch_commits). If this worked as intended, showing candidate versions, or moving tags (nightly), doesn't really matter, so I would not need this feature too in the first place. For the simplicity, not excluding the pre-release tag would be better, as you said and I can fully agree with that. All I wanted to have is to NOT see the (useless) nightly tag; but it turns out that the reason it was showing up is not because it's a pre-release tag that had existed earlier than the full release, but because it's the lexicographically first tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants