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
revelation: init at 0.4.14 #44022
revelation: init at 0.4.14 #44022
Conversation
version = "0.4.14"; | ||
|
||
src = fetchurl { | ||
url = https://bitbucket.org/erikg/revelation/downloads/revelation-0.4.14.tar.bz2; |
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.
You can use string interpolation to reduce the amount lines that need to be changed on updates. E.g.
url = https://bitbucket.org/erikg/revelation/downloads/${name}.tar.bz2;
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.
fixed
|
||
patches = [ ./remove-randpool.patch ]; | ||
|
||
buildInputs = [ pkgconfig gnome2.GConf intltool ]; |
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.
pkgconfig
belongs to nativeBuildInputs
.
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.
intltool maybe as well?
''; | ||
|
||
meta = { | ||
description = "A password manager for the GNOME desktop, released under the GNU GPL license"; |
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.
The "released under the GNU GPL license" is somehow redundant.
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 copied the description from the website, but you are right, it is redundant.
sha256 = "2ab3d1d8bcc2f441feb58122ee6a0fe4070412228194843a180a7b1c9e910019"; | ||
}; | ||
|
||
patches = [ ./remove-randpool.patch ]; |
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.
What is the reason behind removing randpool? Is the patch available upstream? If so please fetch it from there.
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.
The project depends on pycrypt which has been replaced by the successor pycrpytdom in nixpkgs. Randpool is now completely removed from pycryptdom because it was used in an insecure way by many authors. So my choices were to either add the deprecated pycrypt or to apply a patch. The later seemed to be a lot less intrusive.
}; | ||
|
||
# No tests available | ||
doCheck = false; |
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.
Wouldn't it make sense to enable tests for the case upstream adds tests?
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.
The comment was wrong, it does have tests but they depend on a dictionary that is not included in the archive from pypi (probably because it is relatively big).
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.
One commit per package / logical unit
@FRidh Should I open a separate pull request for python-cracklib or are the two commits now alright? |
Adding yourself to the maintainers list needs to be a separate commit as well. |
Are there any updates on this pull request, please? |
Thank you for your contributions.
|
I marked this as stale due to inactivity. → More info |
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)