-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
feishin: 0.3.0 -> 0.4.1 #257430
feishin: 0.3.0 -> 0.4.1 #257430
Conversation
I'm getting a hash mismatch on the downloaded source when I try this:
Is this just something weird with my environment, or maybe did feishin's 0.4.0 tag change since you created the PR? |
Yeah seems to have changed. I meant to update it before i left for conference. Ultimately I'd love for the bot to come through and update tags for us 😂 |
FYI, in case it matter to you personally, there's a bug in 0.4.0 upstream that makes it not consistently submit scrobbles jeffvli/feishin#269 so I'm hoping they'll be making a new new release soon |
Yeah, FYI upstream released 0.4.1 with some bug fixes: https://github.com/jeffvli/feishin/releases/tag/v0.4.1 |
Thanks mate, I'll test and update this PR after work today 😄 |
These are the hash values for anyone who doesn't want to wait for me: filename =
if stdenv.isDarwin then
"${appname}-${version}-mac-x64.zip"
else
"${appname}-${version}-linux-x64.tar.xz";
src = fetchurl {
url =
"https://github.com/jeffvli/feishin/releases/download/v${version}/${filename}";
sha256 =
if stdenv.isDarwin then
"sha256-6GYp9uzlR1eVRYhNU3kOmcUOPFY3J9eJPqN+TucNavA="
else
"sha256-Y8r329rO7z8V2xP/uRsjTFJfvTn+zyeAYzq6fKDxXs4=";
}; |
Don't use merge in PR's here, instead rebase if you have to update the branch. There should only be one commit in this PR, use |
Are these official instructions or your personal opinions as I don't see this in the contributing page? I just followed instructions on the contributing.md to the letter. It states that squashing commits isn't required if they give context to an additional change. |
Not having merge commits is common PR hygiene, rebasing is cleaner when the commit hashes don't have to be preserved and there's a conflict.
https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#commit-conventions This line applies
Having more than one trivial update commit to the same package in a PR is logically unnecessary. |
While I appreciate the advice I really don't think there is an overabundance of commits in here that would confuse/disorientate anyone reviewing this PR. Feel free to make amendments yourself if you feel so inclined. All the required information for a simple version bump is there. |
81f0a76
to
0bfb22a
Compare
I've done it for you, Please remember the advice for your future PRs because other reviewers will say the same. Also fixed a deprecation warning during the desktop file build, and added a forgotten |
``` feishin.desktop> ...feishin.desktop: error: (will be fatal in the future): value item "Audio" in key "Categories" in group "Desktop Entry" requires another category to be present among the following categories: AudioVideo ```
0bfb22a
to
1c7b43d
Compare
Result of 1 package built:
|
Thank you both for updating! |
Description of changes
Simple update to hashes for Feishin version bump.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)