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

Adjust for new asset naming scheme #234

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

dscho
Copy link
Contributor

@dscho dscho commented Aug 12, 2024

As of #1464, the naming scheme of the release assets has changed, breaking this here GitHub Action. Currently, this can only be seen with the nightly variant, but future releases of lychee will see more severe breakages without this PR or something similar.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When the download failed, we should not just go on trying to un-tar a
404 error page.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The name of the `.tar` file is repeated multiple times, verbatim. This
not only violates the Don't Repeat Yourself (DRY) principle, it also
makes it easier to miss an instance when the file name changes (as it
did with lycheeverse/lychee#1464).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Until version 0.15.1, the naming scheme of the release assets let them
start with `lychee-<version>-`. This is no longer the case as of
lycheeverse/lychee#1464, though, breaking the
`lychee-action`.

Since we need to allow using older versions, still, we are now stuck
with a really ugly `case` construct, but this is still far better than
having a broken GitHub Action.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
action.yml Show resolved Hide resolved
@mre
Copy link
Member

mre commented Aug 20, 2024

This looks good.
I've added a proposal to notify users about the ongoing transition, but it turned out to be unnecessary.
This is purely an internal change and should be backwards-compatible.
We can decide to remove the check after a sufficient amount of time to simplify the logic, but there's no user-intervention necessary.

Just to document the behavior for future me, from what I can see, this change covers all four cases:

  1. someone specifies an older version. In this case, we use the old URL for the naming scheme, e.g.
    https://github.com/lycheeverse/lychee/releases/download/v0.15.1/lychee-v0.15.1-aarch64-unknown-linux-gnu.tar.gz
  2. someone specifies a version after 0.15. In this case, we use the new URL for the naming scheme, e.g.
    https://github.com/lycheeverse/lychee/releases/download/v0.16.0/lychee-aarch64-unknown-linux-gnu.tar.gz
  3. someone uses "latest", in which case we use
    https://github.com/lycheeverse/lychee/releases/download/latest/lychee-aarch64-unknown-linux-gnu.tar.gz
  4. someone uses "nightly" in which case the URL is
    https://github.com/lycheeverse/lychee/releases/tag/nightly/lychee-aarch64-unknown-linux-gnu.tar.gz

Thanks for the fix @dscho!

@mre mre merged commit ff9963a into lycheeverse:master Aug 20, 2024
3 checks passed
@dscho dscho deleted the adjust-for-new-asset-naming-scheme branch August 20, 2024 18:17
dscho added a commit to dscho/git-scm.com that referenced this pull request Aug 28, 2024
As of lycheeverse/lychee-action#234, the default
branch of https://github.com/lycheeverse/lychee-action has the fix and I
do not need to refer to a commit that is reachable only in my own fork.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants