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

tedicross: init at 0.8.7 #58096

Merged
merged 2 commits into from Apr 23, 2019

Conversation

@pacien
Copy link
Contributor

commented Mar 22, 2019

Motivation for this change

This PR adds a package and a module for the TediCross Discord-Telegram bridge service.

Current upstream blockers
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 nox --run "nox-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.

@pacien pacien force-pushed the pacien:tedicross-init branch 3 times, most recently from 1e4e1c2 to 10d9784 Mar 22, 2019

@pacien pacien changed the title [WIP] tedicross: init at 0.8.2 tedicross: init at 0.8.3 Mar 22, 2019

@pacien pacien marked this pull request as ready for review Mar 22, 2019

@pacien pacien requested a review from Infinisil as a code owner Mar 22, 2019

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Mar 23, 2019

@GrahamcOfBorg build tedicross

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

Up.

Show resolved Hide resolved nixos/modules/services/networking/tedicross.nix Outdated
Show resolved Hide resolved nixos/modules/services/networking/tedicross.nix Outdated
Show resolved Hide resolved nixos/modules/services/networking/tedicross.nix Outdated

@pacien pacien force-pushed the pacien:tedicross-init branch from f6d2750 to ba24c5e Apr 14, 2019

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

@aanderse Thank you for your review. The PR has been amended accordingly.

@pacien pacien force-pushed the pacien:tedicross-init branch from ba24c5e to f9983f4 Apr 14, 2019

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

Should I also add myself as a maintainer in the module file too?
I don't see a lot of modules having the meta.maintainers property.

EDIT: apparently this has been introduced quite recently. PR amended.

@pacien pacien force-pushed the pacien:tedicross-init branch from f9983f4 to de3c29e Apr 14, 2019

@aanderse

This comment has been minimized.

Copy link
Contributor

commented Apr 14, 2019

The module LGTM 👍

I'm not familiar with node packaging so i'll defer to someone else on that.

@pacien pacien force-pushed the pacien:tedicross-init branch from de3c29e to 32596d5 Apr 14, 2019

@nixos-discourse

This comment has been minimized.

Copy link

commented Apr 14, 2019

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/13

@Infinisil

This comment has been minimized.

Copy link
Member

commented Apr 14, 2019

You should add your package to pkgs/development/node-packages/node-packages-v10.json instead and then call the generate.sh script.

This is also described in the manual: https://nixos.org/nixpkgs/manual/#node.js-packages

@pacien pacien force-pushed the pacien:tedicross-init branch 2 times, most recently from 86d2e6d to d363e51 Apr 14, 2019

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

@Infinisil Thanks for the suggestion. The PR has been amended accordingly.

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

Rebased on master's tip.

@GrahamcOfBorg build nodePackages.tedicross

@pacien pacien force-pushed the pacien:tedicross-init branch from 7b56eb9 to 4044d78 Apr 21, 2019

@pacien pacien changed the title tedicross: init at 0.8.3 tedicross: init at 0.8.5 Apr 21, 2019

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2019

Amended PR to init at 0.8.5 instead of 0.8.3.

@GrahamcOfBorg build nodePackages.tedicross

ExecStart = "${pkgs.nodePackages.tedicross}/bin/tedicross --config='${configYAML}' --data-dir='${dataDir}'";
Restart = "always";
DynamicUser = true;
StateDirectory = builtins.baseNameOf dataDir;

This comment has been minimized.

Copy link
@Infinisil

Infinisil Apr 23, 2019

Member

baseNameOf is in the global namespace, so you can remove the builtins.

This comment has been minimized.

Copy link
@pacien

pacien Apr 23, 2019

Author Contributor

Amended.

@pacien pacien force-pushed the pacien:tedicross-init branch from 4044d78 to 1786723 Apr 23, 2019

@pacien pacien changed the title tedicross: init at 0.8.5 tedicross: init at 0.8.7 Apr 23, 2019

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Amended PR to init at 0.8.7 instead of 0.8.5.

@GrahamcOfBorg build nodePackages.tedicross

@Infinisil

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

Oh one last thing I missed: Can you split it into 2 commits, one for the package, one for the NixOS module? After that I'll merge it

@pacien pacien force-pushed the pacien:tedicross-init branch from 1786723 to d3423dd Apr 23, 2019

@pacien

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

@Infinisil wrote:

Oh one last thing I missed: Can you split it into 2 commits, one for the package, one for the NixOS module? After that I'll merge it

Sure. It's done.

@Infinisil Infinisil merged commit ca37c23 into NixOS:master Apr 23, 2019

13 checks passed

Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details

@pacien pacien referenced this pull request Jun 10, 2019

Open

[WIP] matrix-appservice-irc: init at 0.12.0 #62864

5 of 12 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.