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
glances: move from python-module to application #81192
Conversation
|
||
# fix unitest.py | ||
patches = [ | ||
( fetchpatch { |
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.
looks like a pretty small patch. is there a policy to prefer referencing patches like this, instead of including them directly?
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 has been merged into their master already, which is why I prefer the fetchpatch.
Also communicates that I don't need to upstream the fix
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.
ah, makes sense. maybe link to the upstream issue and mention it can be dropped on the next version bump? :)
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.
correct, oddly enough, this patch was made the same day as the last version. So I'm not sure if he forgot to include it in the release. Either way, it should be dropped next release, I'll make a note of it
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.
works as expected 👍
960ed82
to
0b82d2f
Compare
turns out there was already a glances python module. Now just exporting as a top-level alias |
Since it does not seem to be a python library anyway, this could be moved out of |
@Mic92 true I'll have some free time tonight. I'll do that later |
54b77d5
to
954375d
Compare
954375d
to
68b7779
Compare
@FRidh does this deprecation look right to you? |
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 now :)
Motivation for this change
top/htop alternativeMeant to be used an an application, and not a "library". Moved from python-modules to a top-level entry
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)