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

Jump to bottom

nixos/btrfs autoScrub: don't fail when scrub finishes successfully #84135

Merged
merged 1 commit into from Jun 19, 2020
Merged

nixos/btrfs autoScrub: don't fail when scrub finishes successfully #84135

merged 1 commit into from Jun 19, 2020

Conversation

symphorien
Copy link
Member

Motivation for this change

I noticed that if a scrub finishes successfully ExecStop is still executed and fails because there is nothing to cancel, marking the service as failed.

cc @schmittlauch

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.

@symphorien
Copy link
Member Author

/marvin opt-in

@marvin-mk2 marvin-mk2 bot added the marvin label Jun 16, 2020
@marvin-mk2
Copy link

marvin-mk2 bot commented Jun 16, 2020

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. The stages are

  • needs_review, if the author considers this PR ready
  • needs_work if the PR in its current form is not ready yet. Maybe the reviewer requested changes, there is an ongoing discussion or you are waiting for upstream feedback.
  • needs_merge can be set by reviewers who do not have merge permission but would merge this PR if they could.

Anybody can switch the current status with a comment of the form /status <new_status_here>.

Feedback and contributions to this bot are appreciated.

@symphorien
Copy link
Member Author

/status needs-review

@symphorien
Copy link
Member Author

/status needs_review

mdlayher
mdlayher approved these changes Jun 19, 2020
View reviewed changes
Copy link
Member

@mdlayher mdlayher left a comment

Choose a reason for hiding this comment

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

LGTM!

/status needs_merge

@mdlayher
Copy link
Member

/status needs_merge

@timokau
Copy link
Member

timokau commented Jun 19, 2020

Thanks @symphorien @mdlayher :)

@timokau timokau merged commit 4843eab into NixOS:master Jun 19, 2020
@marvin-mk2 marvin-mk2 bot requested a review from timokau July 3, 2020 20:34
@timokau
Copy link
Member

timokau commented Jul 3, 2020

Oh well, looks like the bot was a bit too enthusiastic here.

@symphorien symphorien mentioned this pull request Jul 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdlayher mdlayher mdlayher approved these changes

@timokau timokau Awaiting requested review from timokau

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 awaiting_changes marvin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@symphorien @mdlayher @timokau