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

Move example path overrides to CI #358

Merged
merged 3 commits into from
Feb 8, 2023

Conversation

stephank
Copy link
Contributor

@stephank stephank commented Feb 5, 2023

Following on #347 (comment), this makes the simple example again test against the checkout, instead of what's in the lockfile. This benefits the direnv test, which can now pick up on any breakages.

I thought this also affected other examples, but noticed they all have a devenv.yaml path override. At the same time, there's a todo to expand devenv init to other examples, so these would also be affected down the line.

This PR removes all those path overrides, and instead adds them in CI right before running the test. This modification makes the lockfiles a little useless, so I've removed them. That also saves a few kb in releases, and helps us catch any breakages via nixpkgs or other inputs.

I also noticed the comment in examples/simple/devenv.yaml was leaking in devenv init. This can be seen in the 0.5.1 release. With this change, we can get rid of it.

@stephank stephank force-pushed the feat/remove-example-locks branch 2 times, most recently from bd2e2c0 to a1c75e2 Compare February 5, 2023 20:21
@stephank
Copy link
Contributor Author

stephank commented Feb 5, 2023

Hmm, removing the lockfiles apparently means we hit GitHub API rate limits.

I'm always mixed on if it's better to test against latest (nixpkgs et al) or if CI stability is preferred. Think I'm coming back on this, and stability is better.

I kept the change moving path overrides to the CI workflow, and updated the lockfile devenv entries.

(cachix/install-nix-action#157 probably helps with rate-limits too, but I think we should be good now.)

@stephank
Copy link
Contributor Author

stephank commented Feb 6, 2023

Huh, CI fails at the nix profile install . step. That seems like a mishap?

@domenkozar
Copy link
Member

Seems like my conflict resolution broke something. Could you revert that and rebase?

@stephank stephank changed the title Remove lock and modules path override from examples Move example path overrides to CI Feb 6, 2023
This allows us to test the `simple` example against the current checkout
too, which also benefits the direnv test. This would've also affected
other examples down the line, when those are added to `devenv init`.
@stephank
Copy link
Contributor Author

stephank commented Feb 7, 2023

Definitely was my fault, simple typo. 😅 I discovered CI works decently well on the main branch of my own fork, so was able to test more there. Simply appending the input to devenv.yaml also doesn't work for simple-remote, because it has an imports section, so I changed it to to insert after inputs: via awk, and that works.

@domenkozar domenkozar merged commit 91d5346 into cachix:main Feb 8, 2023
@stephank stephank deleted the feat/remove-example-locks branch February 8, 2023 07:14
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