Skip to content

Conversation

@alinarielle
Copy link

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 27, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cpvalente
Copy link
Owner

Hi @alinarielle, thank you for this.
I was actually just talking about nix yesterday so it was a fun coincidence to see this PR

I am not too familiar with how nix flakes are used and would need your help here
Is the proposal that users can declare Ontime as a dependency in their setups? Or are we trying to publish Ontime to some nix friendly repo?

Copy link

@dtomvan dtomvan left a comment

Choose a reason for hiding this comment

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

@alinarielle thanks for this contribution! Looking good so far!

@cpvalente A Nix flake makes it easy to install (and with a nixos module in it, configure) some 3rd-party software. The idea is that I declare github:cpvalente/ontime as an input to my own flake with my NixOS configurations in it, and use the provided package definition to install it on my system.

Copyparty does it similarly. Their documentation has a concrete example on how users can make use of the flake if they so desire: https://github.com/9001/copyparty/blob/e9ab040ce8e72e299a3d8fbd109865b1e218eb57/README.md#nixos-module

We could also try to get this into Nixpkgs to make it even more widely available...

}:
{
formatter = pkgs.nixfmt-tree;
packages.default = pkgs.callPackage (
Copy link

Choose a reason for hiding this comment

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

I'd do callPackage ./package.nix {} and put the package in there, because better seperation of concerns and compatibility with non-flake nix (provided you also create a default.nix that also callPackages)

outputs =
inputs@{ flake-parts, ... }:
flake-parts.lib.mkFlake { inherit inputs; } {
systems = [
Copy link

Choose a reason for hiding this comment

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

You could use https://github.com/nix-systems so the systems can be overidden by consumers of the flake.

Copy link

Choose a reason for hiding this comment

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

(nit) I've seen complaints with envrcs being enforced like this (what if you want to, even slightly, modify the environment). It's probably better to allow people to choose what they want in their direnvs, and actually gitignore it.

(while you're at it, because of nix-direnv it might be smart to also gitignore result and .direnv)


inputs = {
flake-parts.url = "github:hercules-ci/flake-parts";
nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
Copy link

Choose a reason for hiding this comment

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

(nit) technically this can just be on tbe nixpkgs-unstable channel since we're not instantiating NixOS configs in this flake. Doesn't matter much though as nixos-unstable only lags behind a couple days due to it having a bit more CI checks and builds.

packages.default = pkgs.callPackage (
{
stdenv,
nodejs,
Copy link

Choose a reason for hiding this comment

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

(nit) Might be best to pin nodejs_22 and pnpm_10 to match the expected engines in the package.json...

@cpvalente
Copy link
Owner

Hi @dtomvan

It seems like you have a good grasp on practices and expected outcomes.
Would you be available to make a proposal with the changes you have suggested?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants