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.

Sign up for GitHub

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

system-config-printer: 1.5.11 -> 1.5.12 #70931

Merged
merged 2 commits into from
Oct 20, 2019
Merged

system-config-printer: 1.5.11 -> 1.5.12 #70931

merged 2 commits into from
Oct 20, 2019
+29 −24

Conversation

symphorien
Copy link
Member

Fixes some dialogs with our version of gtk like

 grid.attach (entry, 1, 2, i, i + 1, 0, 0)
TypeError: Gtk.Grid.attach() takes exactly 6 arguments (8 given)
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @

Sorry, something went wrong.

@worldofpeace worldofpeace self-requested a review October 10, 2019 19:19
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Oct 10, 2019
@teto
Copy link
Member

teto commented Oct 11, 2019

@GrahamcOfBorg build system-config-printer

worldofpeace
worldofpeace suggested changes Oct 11, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Can we update the homepage and document the source url change (in commit message)?
And what purpose can adding packagekit support be in nixos? I'm not sure it looks like it needs it as a buildtime dependency

@symphorien
Copy link
Member Author

@symphorien symphorien force-pushed the system-config-printer branch from 3948eab to f22e933 Compare October 12, 2019 15:56
@symphorien
Copy link
Member Author

Also fedora did this as well: (see changelog for 1.5.11-17):
https://apps.fedoraproject.org/packages/system-config-printer/sources/spec/

I changed the homepage in meta and enabled tests.

@worldofpeace
Copy link
Contributor

1.5.11-17

I wonder why fedora doesn't declare any dependence on packagekit.

packagekit is a runtime dependency $out/bin/install-printerdriver.

So then I guess the system-config-printer nixos module/s needs to enable services.packagekit.enable then. And yes it does need it wrapped with the typelibs

@symphorien symphorien force-pushed the system-config-printer branch from f22e933 to 9ed9baf Compare October 12, 2019 20:28
@symphorien
Copy link
Member Author

Modified the dependency on packagekit in the module (whose existence I discovered with your comment).

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 12, 2019
worldofpeace
worldofpeace suggested changes Oct 12, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Can we default to python3?

@symphorien
Copy link
Member Author

We already use python3: https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/all-packages.nix#L6423

@worldofpeace
Copy link
Contributor

We already use python3: https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/all-packages.nix#L6423

I see. Can we get rid of that? I find it makes the expression hard to read, and overriding shouldn't be any different.

@symphorien
system-config-printer: 1.5.11 -> 1.5.12
a24e9b6 
Source for the change of upstream url:
* zdohnal/system-config-printer is a fork of
OpenPrinting/system-config-printer
* This is what fedora did:
https://apps.fedoraproject.org/packages/system-config-printer/sources/spec/
(see changelog for  1.5.11-17)
@symphorien
nixos/system-config-printer: depends on packagekit
1729506
@symphorien symphorien force-pushed the system-config-printer branch from 9ed9baf to 1729506 Compare October 12, 2019 22:11
@symphorien
Copy link
Member Author

done

@symphorien
Copy link
Member Author

Is there anything that needs to be amended yet ?

@worldofpeace worldofpeace merged commit 64468d4 into NixOS:master Oct 20, 2019
@worldofpeace
Copy link
Contributor

Thanks @symphorien.

@symphorien symphorien deleted the system-config-printer branch March 21, 2020 18:16
@jtojnar jtojnar mentioned this pull request Aug 17, 2022
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@symphorien @teto @worldofpeace