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

onedrive: 2.3.13 -> 2.4.0 #83280

Closed
wants to merge 1 commit into from
Closed

onedrive: 2.3.13 -> 2.4.0 #83280

wants to merge 1 commit into from
+7 −2

Conversation

SRGOM
Copy link
Member

@SRGOM SRGOM commented Mar 24, 2020

Motivation for this change

Upgrade

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.

Sorry, something went wrong.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/best-practice-for-handling-a-breaking-update-to-a-derivation/6365/4

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Mar 24, 2020
@ofborg ofborg bot requested a review from ianmjones March 24, 2020 12:01
@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 Mar 24, 2020
@danbst
Copy link
Contributor

danbst commented Mar 24, 2020

we can start a convention to put some keyword to commit message, which can warrant users about incoming breaking changes. Candidate words: BREAKING, CAVEAT, !!!, (breaking). Commit message should also describe details.

cc @worldofpeace about idea to add such keyword to commit message, for future benefit of unstable users.

@worldofpeace
Copy link
Contributor

we can start a convention to put some keyword to commit message, which can warrant users about incoming breaking changes. Candidate words: BREAKING, CAVEAT, !!!, (breaking). Commit message should also describe details.

cc @worldofpeace about idea to add such keyword to commit message, for future benefit of unstable users.

Doesn't Rust do this and have tooling on top of it?

@danbst
Copy link
Contributor

danbst commented Mar 24, 2020

@worldofpeace sorry, I'm not aware of that. Which tooling?

ianmjones
ianmjones reviewed Mar 24, 2020
View reviewed changes
Copy link
Contributor

@ianmjones ianmjones left a comment

Choose a reason for hiding this comment

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

Builds and runs fine for me, reported same data to sync as previous version after successful --logout and re-auth.

nixos/doc/manual/release-notes/rl-2009.xml
@@ -108,6 +108,11 @@
<link linkend="opt-security.duosec.integrationKey">security.duosec.integrationKey</link>.
</para>
</listitem>
<listitem>
<para>
The package <package>onedrive</package> needs the end user to reauthorize the client for all the accounts. Details <link xlink:href="https://github.com/abraunegg/onedrive/issues/835">here</link>.
Copy link
Contributor

Choose a reason for hiding this comment

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

"here" isn't great context for links.

Can we use something like the following instead?

Details can be found in onedrive's <link xlink:href="https://github.com/abraunegg/onedrive/issues/835">GitHub issue #835</link>.

@ianmjones
Copy link
Contributor

we can start a convention to put some keyword to commit message, which can warrant users about incoming breaking changes.

I feel that this PR doesn't contain breaking changes in the traditional sense. It's more like, users need to be aware that action will be required.

It might have been good if the PR's description had a "Reviewer's Note: ..." or similar, just to double make sure they are aware of the need to re-auth, and maybe mention that it's easy to reset back to previous auth with another --logout once back on the current version.

However, I don't think there's any way of letting true end users know about the requirement to re-auth other than in the release notes is there?

@SRGOM
Copy link
Member Author

SRGOM commented May 26, 2020

@danbst do you want to approve this?

@SRGOM
Copy link
Member Author

SRGOM commented May 26, 2020

Probably too casual, sorry :)

If you can approve this, its better than these changes go in than the automated changes in #86707

This was referenced May 30, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ianmjones ianmjones

@fadenb fadenb

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 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
6 participants
@SRGOM @nixos-discourse @danbst @worldofpeace @ianmjones @fadenb