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

kio-gdrive: init at version 1.2.7 #70754

Closed
wants to merge 12 commits into from

Conversation

konstantinous
Copy link

I had to add signon, signon-ui, kaccounts-integration and providers
packages to be able to build kio-gdrive ( one of the packages from #62168 )

Package works well on my laptop. Signond daemon is probably not able to find it's plugins installed into user profile. However, it doesn't affect kio-gdrive itself.

Motivation for this change

Add google drive integration to KDE dolphin and other kio aware apps.

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.

Copy link
Member

@ttuegel ttuegel left a comment

Choose a reason for hiding this comment

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

I have a few requests, but generally looks good! 😃

pkgs/applications/kde/signon-kwallet-extension.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

signon-ui seems to fail to build

Project MESSAGE: ==== NOTE: Remember to add your API headers into `headers.files' for installation!
Project ERROR: accounts-qt5 development package not found
make[1]: *** [Makefile:44: sub-signon-ui-pro-make_first] Error 3
make[1]: Leaving directory '/build/signon-ui-3acb654/src'
make: *** [Makefile:92: sub-src-make_first-ordered] Error 2
builder for '/nix/store/cx6wbs1lrm47f4jzq09bgpj56xyqs0j7-signon-ui.drv' failed with exit code 2
error: build of '/nix/store/cx6wbs1lrm47f4jzq09bgpj56xyqs0j7-signon-ui.drv' failed

@konstantinous
Copy link
Author

accounts-qt5 is inside buildInputs, so the error is seems weird to me. And it works in my environment... Would you kindly give me more information so I could try to reproduce it?

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/how-to-add-a-new-kde-package-for-kio-gdrive/5539/2

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

each package addition should be it's own commit, and modules commits are generally labeled as:

nixos/module: concise message

maintainers/maintainer-list.nix Outdated Show resolved Hide resolved
@ttuegel ttuegel removed their request for review February 18, 2020 02:13
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Apr 22, 2020

Not exactly, sorry. I meant moving the new changes to the respective package: init at version.
You can do this by doing

  1. a soft reset to HEAD~,
  2. committing each change separately,
  3. interactive rebase (git rebase -i) in which you reorder the commits and use the fixup action to merge the new change with the package: init commit.

The idea is that the history of a PR (like fixing mistakes) shouldn't be preserved when merged into master.

@konstantinous
Copy link
Author

Is there anything else I can do to help the merge happen?

@rnhmjoj
Copy link
Contributor

rnhmjoj commented May 6, 2020

Sorry, I forgot about this PR: unfortunately github doesn't send notification on new commits.
The commits look good now.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented May 6, 2020

@jonringer can you finish your review?

Konstantin added 11 commits May 11, 2020 17:49
Add signong binary in order to use online accounts integration in the future
Add signon-ui binary in order to use KDE online accounts integration in the future
Add oauth2 plugin for signond daemon to authenticate via google api
Add kaccounts-integration package from KDE applications in order to enable online accounts integration
Add kaccounts-providers from KDE applications in order to enable online accounts integration in the future
Add signon-kwallet-extension from KDE applications in order to secure signon information
Add ktp-accounts-kcm from KDE applications in order to enable KDE online accounts integration
Add google drive integration for KDE
It seems the only way to inject variables into user environment since
sidnon daemon requires a path to it's plugins and extensions. By default
it looks for plugins inside the directory in which it's binary is located
Required by kde kcm kaccounts module
@stale
Copy link

stale bot commented Jun 7, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@bachp
Copy link
Member

bachp commented Nov 15, 2021

@konstantinous Are you still working on this? I would like to help you get this merged.

Hoever it needs a rebase and most probably an update of the mentioned components.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 15, 2021
@niknetniko
Copy link
Contributor

niknetniko commented Nov 15, 2021

Hoever it needs a rebase and most probably an update of the mentioned components.

I believe kio-gdrive was added in #122040, so perhaps this is obsolete now.

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

Successfully merging this pull request may close these issues.

None yet

9 participants