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

Jump to bottom

protonvpn-cli-ng: 2.2.2 -> 2.2.4 #91328

Merged
merged 1 commit into from Jul 2, 2020
Merged

protonvpn-cli-ng: 2.2.2 -> 2.2.4 #91328

merged 1 commit into from Jul 2, 2020

Conversation

CloudArtAdmins
Copy link
Contributor

@CloudArtAdmins CloudArtAdmins commented Jun 23, 2020 •
edited

ProtonVPN-cli-ng: 2.2.2 -> 2.2.4

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@CloudArtAdmins CloudArtAdmins changed the title WIP: ProtonVPN-cli-ng: 2.2.2 => 2.2.4 WIP: protonvpn-cli-ng: 2.2.2 => 2.2.4 Jun 23, 2020
@ofborg ofborg bot requested a review from jtcoolen June 23, 2020 03:25
@CloudArtAdmins
Copy link
Contributor Author

So... It still worked without erroring about the hash. I guess it did not change the binary, which is what I assume would change the hash?
Let me know if anything needs to be looked at.

@CloudArtAdmins CloudArtAdmins changed the title WIP: protonvpn-cli-ng: 2.2.2 => 2.2.4 protonvpn-cli-ng: 2.2.2 => 2.2.4 Jun 23, 2020
@jtcoolen
Copy link
Member

You're right. In order to get the new hash, you can change a character in the hash and rebuild the package. Then you'll find the expected hash in the stack trace once the rebuild failed.

@CloudArtAdmins
Copy link
Contributor Author

I ran nixpkgs-review for it.
Ran fine, and didn't error.
Binary works.
Is it possible that the update didn't change the hash of the binary?

@jtcoolen
Copy link
Member

jtcoolen commented Jun 26, 2020 •
edited

No, if you didn't change the hash then it didn't build the new executable. Check its version with -v.
So please change the hash as explained above (it should be 08yca0a0prrnrc7ir7ajd56yxvxpcs4m1k8f5kf273f5whgr7wzw).
Then add the python library jinja2 to the propagatedBuildInputs since it has been introduced in the meantime.

@CloudArtAdmins CloudArtAdmins changed the title protonvpn-cli-ng: 2.2.2 => 2.2.4 WIP: protonvpn-cli-ng: 2.2.2 => 2.2.4 Jun 27, 2020
@CloudArtAdmins
Copy link
Contributor Author

Okay, I see what you mean.
Sorry, I didn't realize that was the case.

@CloudArtAdmins
Copy link
Contributor Author

Okay, got it squashed.
Let me know if there is anything else I should do!

@CloudArtAdmins CloudArtAdmins changed the title WIP: protonvpn-cli-ng: 2.2.2 => 2.2.4 protonvpn-cli-ng: 2.2.2 => 2.2.4 Jun 27, 2020
@jtcoolen
Copy link
Member

Great! Could you rename the commit message to conform to the guidelines?

@CloudArtAdmins CloudArtAdmins changed the title protonvpn-cli-ng: 2.2.2 => 2.2.4 protonvpn-cli-ng: 2.2.2 -> 2.2.4 Jul 1, 2020
@CloudArtAdmins
protonvpn-cli-ng: 2.2.2 -> 2.2.4
53bee28 
Changed version number and added Jinja2 to propagatedBuildInputs.
@CloudArtAdmins
Copy link
Contributor Author

That look about right?

jtcoolen
jtcoolen approved these changes Jul 1, 2020
View reviewed changes
Copy link
Member

@jtcoolen jtcoolen left a comment

Choose a reason for hiding this comment

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

Perfect, thank you!

@jtcoolen jtcoolen requested a review from bhipple July 1, 2020 13:48
@ofborg ofborg bot requested a review from jtcoolen July 1, 2020 19:08
@bhipple
Copy link
Contributor

bhipple commented Jul 2, 2020

Result of nixpkgs-review pr 91328 1

1 package built:
- protonvpn-cli-ng

@bhipple bhipple merged commit 42ea899 into NixOS:master Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhipple bhipple bhipple approved these changes

@jtcoolen jtcoolen Awaiting requested review from jtcoolen

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@CloudArtAdmins @jtcoolen @bhipple