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
pythonPackages.pygls: 0.9.0 -> 0.9.1 #99216
Conversation
4168eef
to
e4a7083
Compare
@@ -1,30 +1,29 @@ | |||
{ stdenv, buildPythonPackage, isPy3k, fetchFromGitHub | |||
, mock, pytest, pytest-asyncio | |||
{ stdenv |
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.
Instead of stdenv
consume lib
directly, you don't need stdenv
anywhere.
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.
Thanks. I've been asked to use stdenv.lib
instead of just lib
in previous code reviews, but I prefer using lib
, so I will update it here.
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.
I believe stdenv.lib
is a copy/paste artifact, but someone more experienced may be able to clarify.
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.
It wasn't just an artifact, I was explicitly asked to use stdenv.lib
instead of just lib
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.
I updated my commit, but I'll leave this as unresolved if anyone else wants to leave comments.
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.
@mweinelt IIRC, it was just a convenience to add lib to stdenv.
e4a7083
to
e96420d
Compare
Result of 3 packages built:
|
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
Result of nixpkgs-review pr 99216 1
3 packages built:
- cmake-language-server
- python37Packages.pygls
- python38Packages.pygls
Motivation for this change
Upgrade to the latest version: https://github.com/openlawlibrary/pygls/releases/tag/v0.9.1
Things done
Tested using sandboxing (nix.useSandbox on NixOS, or option
sandbox
innix.conf
on non-NixOS linux)Built on platform(s)
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/
)No binary files
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.