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

etesync-web: init at 0.6.1, nixos/etesync-web: init module and test #130680

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

etesync-web: init at 0.6.1, nixos/etesync-web: init module and test #130680

wants to merge 3 commits into from

Conversation

Pacman99
Copy link
Contributor

@Pacman99 Pacman99 commented Jul 20, 2021 •
edited
Loading

Motivation for this change

Provide the official web ui for etesync and make it easy to setup.

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/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions 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 Jul 20, 2021
@Pacman99 Pacman99 requested a review from ryantm as a code owner July 20, 2021 00:43
@github-actions github-actions bot added 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Jul 20, 2021
@Pacman99 Pacman99 force-pushed the etesync-web branch 3 times, most recently from 8da3406 to a405e21 Compare July 20, 2021 00:58
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Jul 20, 2021
@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 130680 at a405e214 run on aarch64-linux 1

2 packages built successfully:
  • etesync-web
  • nixos-install-tools

dotlambda
dotlambda reviewed Jul 20, 2021
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

I would be very reluctant to accept 24k new lines for a single package.

nixos/modules/services/web-apps/etesync-web.nix
};
defaultServer = mkOption {
type = with types; nullOr str;
default = if etebaseHost != "0.0.0.0" then etebaseHost else null;
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this line? Why is 0.0.0.0 not a valid value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll clarify with a comment. But the idea is that etebase-server's default allowed_host1 is 0.0.0.0, so I'm only setting the etebase host if its not the default.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is actually useful, since it won't cause etesync-web to be rebuilt for the default setup.

Copy link
Member

Choose a reason for hiding this comment

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

I suggest default = "0.0.0.0". The resulting package will be cached anyway due to the test.

Copy link
Contributor Author

@Pacman99 Pacman99 Jul 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I suggest default = "0.0.0.0". The resulting package will be cached anyway due to the test.

Thats not a working or sane default. If you enable etesync-web without an etebase server then the best default is the official etebase server instance(etesync.com). That way etesync-web will actually work without setting a server. 0.0.0.0 doesn't really mean anything.

@Pacman99
Copy link
Contributor Author

I would be very reluctant to accept 24k new lines for a single package.

Thats not really fair, the standard way to package yarn2nix without IFD involves committing the lockfile and yarn2nix output. I'm just followed the standard, your concern is more about nixpkgs semantics itself.

SuperSandro2000
SuperSandro2000 requested changes Jul 20, 2021
View reviewed changes
pkgs/servers/web-apps/etesync-web/default.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/etesync-web/default.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/etesync-web/default.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/etesync-web/default.nix Outdated Show resolved Hide resolved
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 20, 2021
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 20, 2021
@stale
Copy link

stale bot commented Apr 19, 2022

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 Apr 19, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 22, 2022
@Pacman99
Copy link
Contributor Author

Still relevant to me

@SuperSandro2000
Copy link
Member 

trying https://registry.yarnpkg.com/wait-for-expect/-/wait-for-expect-3.0.2.tgz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  9368  100  9368    0     0   411k      0 --:--:-- --:--:-- --:--:--  415k
100  343k    0  343k    0     0   276k      0 --:--:--  0:00:01 --:--:-- 1703k
unpacking source archive /private/tmp/nix-build-source.drv-0/v0.6.1.tar.gz
hash mismatch in fixed-output derivation '/nix/store/qwc3hdwa9n9chnx97bjayhmal8sin0wg-source':
  wanted: sha256:12y9syl159q71xqa0dcsxc2ir5y8yi5rs51p9f7h36h8imnljhw9
  got:    sha256:1mnxp7s32kx6hx5faviq06zsa8596y6bidlfs40vg3q389w5n2k5

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 30, 2022
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@donovanglover donovanglover marked this pull request as draft February 6, 2025 09:11
@donovanglover donovanglover changed the title Etesync web etesync-web: init at 0.6.1, nixos/etesync-web: init module and test Feb 6, 2025
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

@matt-snider matt-snider Awaiting requested review from matt-snider

@felschr felschr Awaiting requested review from felschr

@ryantm ryantm Awaiting requested review from ryantm

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 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 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 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.
6 participants
@Pacman99 @r-rmcgibbo @SuperSandro2000 @felschr @dotlambda @wegank