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

lunar-client: migrate to by-name and add updateScript #263015

Merged
merged 2 commits into from
Nov 6, 2023

Conversation

surfaceflinger
Copy link
Member

Description of changes

as in title.
also please lmk if this script will work with r-ryantm bot :)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@OPNA2608 OPNA2608 left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 263015 run on x86_64-linux 1

1 package built:
  • lunar-client

Hash & fetched binary works. Update script tested by setting version to 3.0.0 and running it, which correctly reset everything to the values in this PR. LGTM.

Was trying my hardest to see if update-source-version could be used instead of seding around in the package file, but it can't handle appimageTools.wrapType2 -> appimageTools.wrapAppImage -> buildFHSEnv not setting a version 🙁.


also please lmk if this script will work with r-ryantm bot :)

If it works for nix-shell maintainers/scripts/update.nix --argstr package lunar-client, then it should work for the update bot.

@surfaceflinger
Copy link
Member Author

Was trying my hardest to see if update-source-version could be used instead of seding around in the package file, but it can't handle appimageTools.wrapType2 -> appimageTools.wrapAppImage -> buildFHSEnv not setting a version 🙁.

yeah, I spent quite lots of time trying that already

@OPNA2608
Copy link
Contributor

OPNA2608 commented Nov 5, 2023

(I guess this package is also lacking meta.mainProgram, but again not a deal breaker for this PR)

(and meta.description is not supposed to have a trailing .))

Copy link
Member

@pbsds pbsds left a comment

Choose a reason for hiding this comment

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

You can test the updater script with nix-update lunar-client -u

pkgs/by-name/lu/lunar-client/update.sh Show resolved Hide resolved
@pbsds
Copy link
Member

pbsds commented Nov 6, 2023

Result of nixpkgs-review pr 263015 run on x86_64-linux 1

1 package built:
  • lunar-client

Builds and runs fine, tested the update script. LGTM

@pbsds pbsds merged commit 680d005 into NixOS:master Nov 6, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants