-
Notifications
You must be signed in to change notification settings - Fork 71
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
Re-enable basic CI #239
base: master
Are you sure you want to change the base?
Re-enable basic CI #239
Conversation
This will probably require some changes in the repo settings to enable CI and have a Cachix token, but I do not have access to that, so you'll have to do it after it's merged. |
Also, possibly outstanding questions: a) I wanted to add formatting for pipeline YAMLs, but apparently |
a) Ugh, what an arbitrary l limitation. That and the fact that it's so tightly coupled to flakes makes me want a different tool. How hard can it be to run a formatter on a bunch of files? |
What would we need cachix for? We won't be building anything in GH actions. |
Fair, it's an annoying limitation (didn't know if it beforehand, sorry) and it's not very hard to just run formatters — but having a consistent interface for multiple formatters that runs them in parallel and properly reports errors is a wee bit harder, which is why I decided to go with a pre-existing solution. Of course it turns out it's not perfect xD I'd personally be willing to overlook pipeline formatting until they fix that but it seems they are somewhat slacking on maintenance (a version with the
Is it, though? I was under impression it can be used just the same from classic Nix (at least
Sure, will add some prose to this and #237.
Sorry if I wasn't clear here — this is something that would have to be toggled in the repository Settings tab (c.f. GitHub docs). I do not have access to that, so someone else will have to do that after this PR is merged. I'm just pointing out that this PR won't help a lot until it will be enforced by setting required checks up.
YMMV I guess, because I'm the reverse — but maybe that's because I'm a shit developer xD More seriously though, bad word choice — linear history is tangential to what I wanted to say. What I did really mean is having branch up to date when running tests — either via GH forcing people to update their branches before merging (which I conflated with linear history, because I usually update via rebase) or having a merge queue. Because without ensuring the branch is up to date you're liable to end up with a situation where tests pass on branches, but after they are merged together into the master branch, it starts failing due tologically conflicting changes. While maybe there could be some argument made to living with that for the sake of development velocity (which I don't really agree with, but that's just differing priorities, I guess), I'm not sure if it applies to robotnix where maintenance time is very limited, so the master branch could be failing for a long time before someone has time to fix it and then review it. Which is why I raised the issue, because I think setting up something that stops it from happening would be beneficial.
Not in this PR, no. But my impression was that there were thing being built in CI before (https://github.com/nix-community/robotnix/blob/master/release.nix) and I'd assume we would eventually do it again when things are building consistently again (and given it would, like the above, require someone else to muck around in Settings, I figured it'd be better to set up both things at once). Having kernels and browsers in a cache is certainly better than having to build them locally and I kind of assumed that if robotnix had done so previously, it's not problematic to do it again. If for some reason it's a problem to do it again, then I guess it could go. But at this point I'm not entirely sure what the benefits of upstreaming things would be, if everyone will have to build and check nothing is broken on their own machines anyway. |
Re: alternative tooling - I could try and see how https://github.com/cachix/git-hooks.nix works can be used for this, just don't enable githooks in the default shell, if you don't want that to be the default. Could that be an option? |
Formatter problems should be resolved now: #237 (comment) |
8f8c72e
to
2bb46e2
Compare
This re-enables basic CI checks:
a)
treefmt
,b)
pytest
,c)
nix flake check
,d)
nix flake check
for the flake generated from the flake template.It would probably be useful to fix and re-enable more checks, but I think it's better to have those basic ones in earlier and do others as a follow-up.
I have also took the opportunity to add direnv support for convenience and moved flake outputs to a separate file in
./flake
so it's a bit cleaner. Let me know if you (don't) mind.Based on the top of #237, so it needs to go first.