Skip to content

First use tweaks#125

Open
ThinkChaos wants to merge 5 commits intonix-community:mainfrom
ThinkChaos:first-use-tweaks
Open

First use tweaks#125
ThinkChaos wants to merge 5 commits intonix-community:mainfrom
ThinkChaos:first-use-tweaks

Conversation

@ThinkChaos
Copy link

@ThinkChaos ThinkChaos commented Dec 21, 2023

Hi, thanks for building this and haumea, both are cool projects!

Here's a couple changes I did when getting setup. If you'd like me to split this in any way, let me know.
All changes are pretty small though.

  1. fix: interpret path printed by nix as relative to repo root

    This was causing check to always see the tests as new/failing, but
    review to show a diff with no changes.

    My setup is similar to the subflake template, so it's probably worth adding automated tests for the templates as that would've probably caught this bug.

  2. feat(check): track test additions separately from failures

    Tweak the output so those cases are more easily distinguishable,
    especially the color used by the + symbol next to the test name.
    Exit code is now 2 if all existing tests succeeded, but some were added.

    Here's an example output in the common case:
    image
    Here's a comparison of "failing and added state" which is what motivated me to change this:
    image
    I was seeing the above a lot because of 1.
    image

  3. fix: only show flake and dir warning when relevant

    Forgetting to set src was triggering it.
    Ideally we'd just change the function to use { src, ... }@args: so Nix
    can provide an explicit error the users are accustomed to.

This was causing `check` to always see the tests as new/failing, but
`review` to show a diff with no changes.
Tweak the output so those cases are more easily distinguishable,
especially the color used by the `+` symbol next to the test name.
Exit code is now 2 if all existing tests succeeded, but some were added.
Forgetting to set `src` was triggering it.
Ideally we'd just change the function to use `{ src, ... }@Args:` so Nix
can provide an explicit error the users are accustomed to.
Copy link
Member

@figsoda figsoda left a comment

Choose a reason for hiding this comment

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

Sorry for the super late review

1: I can't reproduce this error. Do you have the snapshots added to git? If this fixes the issue, maybe namaka can automatically add the snapshots to git (like nix does with flake.lock)?

Everything else lgtm, I'm happy to revert (1) and merge everything else as-is.

Co-authored-by: figsoda <figsoda@pm.me>
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.

2 participants