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.

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

Add kdeapps #98212

Open
wants to merge 50 commits into
base: master
from
Open

Add kdeapps #98212

wants to merge 50 commits into from

Conversation

@freezeboy
Copy link
Contributor

@freezeboy freezeboy commented Sep 18, 2020

Motivation for this change

Continuing the process to make all applications in applications/kde/srcs.nix, I used the same technique as the previous PR:

  • one commit per lib/app
  • export user facing apps in top-level/all-packages.nix

A few notes:

  • I had to modify the accounts-qt package as the structure of the out was fooling cmake
  • I added signond as it contains the dbus information required for kaccounts-integration package, but didn't take care to enable a service for it
  • Dev apps (kdesdk, etc) are only in kdeApplications namespace

A few kde apps are still not packaged, three I didn't succeed yet (cervisia, signon-kwallet-extension and artikulate), kopete as it is obsolete upstream and all the telepathy stack

Maybe we should push this to staging ?

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.
@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 30, 2020

@freezeboy your work is valuable, but I'm afraid it may not worth the maintainence effort to put all of these packagegs in our collection. Do you think you can add here only those that you personally would like to use?

@freezeboy
Copy link
Contributor Author

@freezeboy freezeboy commented Oct 30, 2020

What do you mean in "maintainance effort"? basically all the sources are updates in bulk with the srcs.nix file for kde, I just thought it was sad not to provide packages when it is just a matter of listing the deps of each app.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 30, 2020

Listing the deps and maintaining them is the maintenance. Personally I do tend to think the more is the better, but @ttuegel 's approval might be needed for this. Let's hope he'll find time to review this.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 30, 2020

20.08.2 is out in the meantime..

@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Nov 3, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/272

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Nov 3, 2020

@freezeboy there are merge conflicts, and I'd post this PR in the "ready for review" thread.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Nov 3, 2020

It should be a rebase instead of a merge master into your branch. Also the commit messages should be init at 20.08.2. You don't have to separate all of them into separate commits IMO.

@freezeboy freezeboy force-pushed the freezeboy:add-kdeapps branch from 2d92c35 to eb37c5d Nov 3, 2020
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Nov 15, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/368

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.