-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
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
glimpse: init at 0.2.0 #103288
glimpse: init at 0.2.0 #103288
Conversation
340b839
to
9336a5c
Compare
0ba253e
to
02213e5
Compare
Just pushed some more changes to |
02213e5
to
735680f
Compare
@jtojnar The more I think about it the lesser I am convinced that maintaining a separate Maybe {
# simple case for just swapping gimp with glimpse
gap = gimpPlugins.gap.override { gimpPackage = glimpse; };
# complex case where overriding makes no sense and an entirely new plugin is constructed.
fourier = gimpPlugins.buildGimpPlugin {
pname = "fourier";
version = "0.4.3";
gimpPackage = glimpse;
postPatch = ...;
};
} Another way could be to keep What do you think? I have not much insight into this topic, so I guessed I'd ask you first before trying to implement it. |
735680f
to
07e29a6
Compare
Thanks, I think that does the job. Much less code now. Depends now on #103361 |
07e29a6
to
9d57f9e
Compare
@jtojnar as you already did quite a bit ot review work, would you mind taking another look on this PR? Or maybe @ashkitten as the original author? Would be nice to get this merged eventually. |
@erictapen please fix the merge conflict. |
9d57f9e
to
031782b
Compare
Fixed the conflict. Took me a while, but glimpse (as well as gimp) need autoconf 2.69. |
031782b
to
75183d3
Compare
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 would appreciate if @jtojnar could take another look at this.
Seems fine from my side.
75183d3
to
e0d1293
Compare
Multiple people eyeballed this PR and I'm going to maintain all of its code anyway. So without further objections I'll merge this in two days. |
Co-authored-by: Louis Bettens <louis@bettens.info> Co-authored-by: ash lea <example@thisismyactual.email>
e0d1293
to
ae6023b
Compare
This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 2 packages failed to build and are new build failures:
1 package built:
|
This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 4 packages marked as broken and skipped:
18 packages built:
|
Motivation for this change
Closes #77157
Succeeds #81563
Pinging @grahamc @jtojnar as they already started reviewing #81563.
Thanks to @ashkitten @lourkeur for previous work.
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)