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

Rename example to tests #849

Open
Mic92 opened this issue Oct 26, 2024 · 7 comments
Open

Rename example to tests #849

Mic92 opened this issue Oct 26, 2024 · 7 comments
Labels
testing Missing or incorrect tests

Comments

@Mic92
Copy link
Member

Mic92 commented Oct 26, 2024

We should link users to https://github.com/nix-community/disko-templates/
Our examples are in fact tests.
In disko-templates we can than also start testing examples against a stable version of disko and not have to run example tests on every pull request, which can be potentially slow.

@iFreilicht
Copy link
Contributor

Why? I feel like it's a very good thing that all our examples are tests. This means that no matter what's on master, all examples are always guaranteed to work, and we can merge PRs with confidence.

I would much rather delete disko-templates outright, there seems to be no advantage to me of keeping examples in a separate repository.

If we absolutely want users to be able to use nix init -t, we can add the examples as templates here as well, though I personally don't think that's any more useful that telling users to download something with curl.

@Mic92
Copy link
Member Author

Mic92 commented Oct 26, 2024

The original idea was that templates could be added without creating a new release. And examples would than target stable releases rather than unstable.

@iFreilicht
Copy link
Contributor

Okay that makes sense as a historical explanation, but do you think that is still an important goal? From my PoV it seems we can just do patch releases for new templates. We only merge changes to master that we deem releasable anyway.

@Lassulus
Copy link
Collaborator

Lassulus commented Nov 5, 2024

Our examples currently contain a lot of technical details that aren’t typically needed in a standard Disko configuration. It would be helpful if the examples showcased simpler use cases by default, moving the more complex cases currently covered by examples into dedicated tests. We could also consider supporting templates, though keeping them external for now should be sufficient. In the future, we might integrate these templates directly into the repository. I’d also like to develop an external module that provides streamlined Disko configurations with a versioning schema to ensure compatibility, but that's a task for later.

@Mic92
Copy link
Member Author

Mic92 commented Nov 5, 2024

I would be fine having them in the main repo these days. Only concern is that I don't want to limit contributors how many templates they want to add. If we have to run tests on them, than we make contributing to our project eventually slower.

@iFreilicht
Copy link
Contributor

It would be helpful if the examples showcased simpler use cases by default, moving the more complex cases currently covered by examples into dedicated tests.

What brings you to this conclusion? Personally I often feel like examples being too simple makes it harder to learn about how to use a software to its full potential. I am very happy that we have examples that cover even advanced usecases and showcase technicalities that might get in your way.
Has any user complained about the examples being too complex or too hard to understand? When an issue is created here due to lack of understanding, is there any indication this lack of understanding stems from our examples being too complex?

In the future, we might integrate these templates directly into the repository. I’d also like to develop an external module that provides streamlined Disko configurations with a versioning schema to ensure compatibility, but that's a task for later.

What does this mean? What are "streamlined" Disko configurations? Why is it better to have them in an external repo?

I would be fine having them in the main repo these days.

Okay great, I agree with that.

Only concern is that I don't want to limit contributors how many templates they want to add. If we have to run tests on them, than we make contributing to our project eventually slower.

I don't think we need to be concerned about this. First, untested templates might not work, and non-working templates are worse than none at all, so I'd rather just have no templates contributed than untested ones. Second, I don't see how testing has slowed down contributions to our examples compared to disko-templates.

I would be fine with templates being tested less thoroughly than the examples, but at the very least it should be checked if they evaluate. Though to be honest, when templates contain things like a postCreateHook, we should really do a VM test.

@iFreilicht iFreilicht added the testing Missing or incorrect tests label Nov 6, 2024
@Lassulus
Copy link
Collaborator

The problem is, that people sometimes copy the fancy features from the examples because they think they need them (cargo culting?). So easier examples expose less complexity and people could understand them easier after they have a minimal config with a running system :)

streamlined config is something I though about in the past. maybe something like:

easyDisko.zfsRaid = {
  enable = true;
  level = "zraid5";
  devices = [
    "/dev/disk/by-id/samsung1"
    "/dev/disk/by-id/samsung2"
    "/dev/disk/by-id/samsung3"
  ];
  settings = {
    compression = true;
    dedup = true;
  };
};

since these expose a lot of modules and opinionated code, it would be easier to have them out of tree. The complexity of these modules is also something that should be tested more thoroughly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Missing or incorrect tests
Projects
None yet
Development

No branches or pull requests

3 participants