Skip to content

onedrive: 2.3.12 -> 2.3.13 #77854

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
Merged
merged 1 commit into from
Jan 20, 2020
Merged

onedrive: 2.3.12 -> 2.3.13 #77854

merged 1 commit into from
Jan 20, 2020
+11 −19

Conversation

ianmjones
Copy link
Contributor

@ianmjones ianmjones commented Jan 17, 2020 •
edited
Loading

Motivation for this change

Upgrade OneDrive to latest version.

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 Hope this is ok to submit as a PR and add myself as a maintainer.

I've been using OneDrive with an almost identical Nix file for a couple of months via an overlay, and just as I went to add it to nixpkgs found your version!

This is an update to the latest release, adds in the ability to possibly get notifications by compiling with that option, and runs the default.nix file through nixfmt to help standardise the format.

Sorry, something went wrong.

@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 Jan 17, 2020
@ofborg ofborg bot requested review from doronbehar and SRGOM January 17, 2020 00:52
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 17, 2020
@alyssais alyssais changed the title onedrive: 2.3.12 -> 2.3.13 onedrive: 2.3.12 -> 2.3.13; nixos/onedrive: init Jan 17, 2020
@alyssais
Copy link
Member

I’ve changed the pull request title to make it clearer that this is not just a minor package bump. Can you please split your commit into two, one two bump the package, and another to add the module?

@alyssais
Copy link
Member

Is your module actually usable? Have you tested it? I ask because it hasn’t been added to module-list.nix.

@alyssais alyssais added 8.has: module (new) This PR adds a module in `nixos/` and removed 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 17, 2020
@SRGOM
Copy link
Member

SRGOM commented Jan 17, 2020

I have created another module here #77734 which allows running OneDrive services for multiple accounts. The version bump is fine. I'm not
too familiar with code guidelines.

@ianmjones ianmjones requested a review from infinisil as a code owner January 17, 2020 09:57
@ofborg ofborg bot added the 8.has: module (update) This PR changes an existing module in `nixos/` label Jan 17, 2020
@ianmjones
Copy link
Contributor Author

@alyssais Thanks for taking a look at this PR and giving me some hints, much appreciated.

I've split the version bump and module addition into two commits.

I've tested this locally, just forgot to add that change to module-list.nix to git, it's in that second commit now, sorry about that!

@SRGOM Oops, didn't see that PR, looks interesting, I'll leave some comments/questions on it in a bit.

@ianmjones ianmjones mentioned this pull request Jan 17, 2020
Closed
@doronbehar
Copy link
Contributor

Hey all, I'm going to be really busy at the next few weeks, and I have little to no interest at all in this package or service, so I don't have time to review this. Sorry.

@ianmjones
Copy link
Contributor Author

@doronbehar No worries, shall I take you out of the maintainers list as part of the version bump?

@alyssais & @SRGOM I'm thinking I should remove the module stuff from this PR and we should just go with #77734, sound good?

If so, would be great if you could maybe add the cfg.package stuff to your PR @SRGOM to better enable alternate package versions to be used etc.

@alyssais
Copy link
Member

alyssais commented Jan 17, 2020 via email

@SRGOM
Copy link
Member

SRGOM commented Jan 17, 2020

@ianmjones I have added cfg.package as you suggested in #77734

@doronbehar
Copy link
Contributor

@ianmjones yes, please remove me from the maintainers list.

@ianmjones ianmjones changed the title onedrive: 2.3.12 -> 2.3.13; nixos/onedrive: init onedrive: 2.3.12 -> 2.3.13 Jan 17, 2020
@ofborg ofborg bot removed 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 Jan 17, 2020
@ianmjones
Copy link
Contributor Author

ianmjones commented Jan 17, 2020 •
edited
Loading

@alyssais Thanks, I've removed the module, so it's just a version bump with @doronbehar removed from the maintainers as requested.

@alyssais alyssais merged commit 96d2790 into NixOS:master Jan 20, 2020
@ianmjones ianmjones deleted the onedrive-2.3.13 branch January 20, 2020 19:09
@ianmjones
Copy link
Contributor Author

Thanks @alyssais!

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/steps-to-find-a-commit-with-cached-version-of-package/5674/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doronbehar doronbehar

@SRGOM SRGOM

@infinisil infinisil

Assignees
No one assigned
Labels
8.has: module (new) This PR adds a module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ianmjones @alyssais @SRGOM @doronbehar @nixos-discourse