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

cryptpad: init at v2.23.0 #61513

Merged
merged 2 commits into from Jun 7, 2019
Merged

cryptpad: init at v2.23.0 #61513

merged 2 commits into from Jun 7, 2019

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented May 14, 2019 •
edited

Motivation for this change

This project is part of the https://nlnet.nl/NGI/ grant.

The project still needs a bit of work to be ready to be merged:

  • it's using bower which is deprecated by upstream
  • it's tested with node 6.x (EOL) in the Dockerfile
  • it assumes that the source folder is writable, it uses the source folder to store it's state
  • it assumes that the configuration file is stored in a fixed location in the source root
  • it assumes that the ./server.js entry point is executed from the source root
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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@zimbatm zimbatm added the 6.topic: NLNet / NGI The project is part of the NGI funding grant. https://nlnet.nl/NGI/ label May 14, 2019
@zimbatm zimbatm mentioned this pull request May 14, 2019
Merged
@zimbatm zimbatm mentioned this pull request May 15, 2019
Open
@zimbatm zimbatm force-pushed the cryptpad-2.21.0 branch 2 times, most recently from 03e664c to a0a3126 Compare May 15, 2019 14:11
@zimbatm zimbatm changed the title WIP: cryptpad: init at v2.21.0 cryptpad: init at v2.21.0 May 15, 2019
@zimbatm zimbatm marked this pull request as ready for review May 15, 2019 14:18
@zimbatm zimbatm requested a review from infinisil as a code owner May 15, 2019 14:18
infinisil
infinisil requested changes May 15, 2019
View reviewed changes
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Review for the NixOS module

nixos/modules/services/web-apps/cryptpad.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/cryptpad.nix Show resolved Hide resolved
nixos/modules/services/web-apps/cryptpad.nix
enable = mkEnableOption "the Cryptpad service";

package = mkOption {
default = pkgs.cryptpad;
Copy link
Member

Choose a reason for hiding this comment

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

I personally feel like we shouldn't add package options when there's no need to, but not everyone feels the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok let me contribute to that other thread then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like I already commented: #50476 (comment)

nixos/modules/services/web-apps/cryptpad.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/cryptpad.nix Outdated Show resolved Hide resolved
@zimbatm zimbatm requested a review from infinisil June 7, 2019 11:06
@zimbatm zimbatm changed the title cryptpad: init at v2.21.0 cryptpad: init at v2.23.0 Jun 7, 2019
@zimbatm zimbatm merged commit d838603 into NixOS:master Jun 7, 2019
@zimbatm zimbatm deleted the cryptpad-2.21.0 branch June 7, 2019 12:28
@zimbatm zimbatm mentioned this pull request Jun 7, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
6.topic: nixos 6.topic: NLNet / NGI The project is part of the NGI funding grant. https://nlnet.nl/NGI/ 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@zimbatm @infinisil