-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
warp-terminal: added passthru update script #292928
Conversation
d3d05f3
to
a6a310f
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/does-update-source-version-have-a-generic-system-flag/40679/1 |
dfac750
to
5234ea1
Compare
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.
change looks good. minor comments inline.
do you want to add yourself to the maintainers?
Yeah, I'd like to be a maintainer if my other PRS aren't going to be merged before this. I'll update the other ones if this gets merged. |
add the maintainer commit to this PR as this PR is close to getting merged. you need to fix your other commit message though. the body is not adding anything useful to someone who will be looking at the commit logs sometime in the future. look at the output from |
23e3554
to
df6d07f
Compare
I've added myself to the maintainer list and removed my commit body - I was squashing instead of amending and forgot to remove the commit messages getting added to the body. |
df6d07f
to
bbd653d
Compare
there are some somewhat strict rules about commit messages. the commits need to be: the maintainers needs to come first, otherwise if someone is performing a bisect the eval will break if they land on the |
bbd653d
to
aa9b78d
Compare
I'm gonna pretend it was because I didn't drink coffee today and nothing else. I think it should be good for real this time |
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.
LGTM. validated curl -s https://api.github.com/users/imadnyc|jq '{name,github:.login,githubId:.id}'
matches github info in maintainers
version=$(get_version "${url}") | ||
if [[ ${version} != "$(json_get ".${sys}.version")" ]]; | ||
then | ||
sri=$(nix hash to-sri --type sha256 "$(nix-prefetch-url --type sha256 "${url}")") |
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.
haven't seen an update posted yet. this might need to be nix-hash --to-sri
as nix hash
is experimental and can generate an error if the magic config settings are not set.
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.
or not -- i see nix hash
in some other update.sh. pkgs/tools/video/recyclarr/update.sh
and looks like it had an auto-update. #267039
and the update script has not run yet on warp-terminal. https://r.ryantm.com/log/warp-terminal/
Description of changes
Finally got around to adding a passthru update script. Here, I manually updated the macOS hash because to my understanding
update-source-version
expects a platform+architecture, but doesn't accept a general platform, which warp does with it's.dmg
(again, to my understanding, I don't have a mac). I also wanted to add myself as a maintainer of this, but I already have that commit in #291443, and didn't want to duplicate the commit. If this is ready to be merged before that, though, I'll remove that one and add it here.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.