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

Proposal: Nix Powered Checks #141

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JayRovacsek
Copy link
Member

Background

Creating this as only a proposal for now; as I had written the majority of code required for this change-set in #134 I figured I'd create it anyway.

This changeset could be seem as covering #90 but has a caveat that the underlying technology is generally a unix-only option leading to a need to either:

  • document how a user would utilise this approach
  • throw the PR away as it is at odds with keeping to simpler technology choices

Why

Why nix when it is known to have a reasonable learning curve?

Non-Blocking

If considered suitable; I would suggest we keep the check run as not-required and instead it can provide value by providing feedback on how adherent a PR is to the listed checks above.

Reproducibility

Nix enables reproducibility rather than just repeatability; if the checks pass on your machine utilising this mechanism, it'll absolutely pass within the checks flow also, the only possible caveat to this will be if a macOS or Windows opinion differs from Linux; only as the checks run will run under Linux;

Between macOS and Linux I've never seen this eventuate. I cannot speak for Windows; but in the Windows case I believe the only reasonable approaches to run this would be via docker anyway.

Enabler of Auto-Merge

Currently the repository does not have auto merges enabled; this could be utilised as an approach to ensuring rigour of a changeset that enables this feature to be enabled

Features

In essence, the added flake.* files and .envrc allow a user who has nix installed to run nix flake check to ensure all code adheres to checks described here:

  • actionlint - ensures github action file suitability
  • deadnix - ensures no code within *nix files that are evaluated is unused or proposes removing it
  • nixfmt - RFC candidate for the nix language
  • prettier - across all suitable files in the repository
  • statix lint and recommends best practice for *nix code (plus the custom write check I've written enforces those opinions)
  • trufflehog - custom hook that will attempt to identify and validate secrets committed into the repository

How

These checks can be adopted into a workflow via direnv running or via nix develop which will install the hooks at shell start time and provide the listed packages also as an ephemeral shell.

Examples

An example of all checks passing is as per this actions run, if ran locally it would emit the following when in verbose mode:
image

Further

An example of utilising this integrated with git hooks (change on the right is the modification the hook as applied, left is git error log)
image

Treefmt is supported by the underlying git-hooks.nix repository and might be a good option to centralise all checks if this change-set is considered suitable.

Next Steps

Feel free to provide feedback; good and bad; I'm happy to enable conversation to clarify anything required on the above ❤️

@mtranter
Copy link
Contributor

My 2c: Nix will increase the barrier to entry for juniors (and probably most devs - including me 🤣 )
Should we not optimise the stack to be as inviting as possible for the junior folk in the community?

@JayRovacsek
Copy link
Member Author

My 2c: Nix will increase the barrier to entry for juniors (and probably most devs - including me 🤣 ) Should we not optimise the stack to be as inviting as possible for the junior folk in the community?

I'd generally agree - happy to consider a best of a few worlds if we agree also:

Merge but make non-blocking + keep original issue open; this would still enable someone in the community to make a proposed changeset that brings the idea back down to a more accessible option. I did consider making a proposal via husky or pre-commit but both are going to be trade-offs also to an extent 😬
We could remove this when we get a more favourable changeset?

I could accept also the idea that this is simply too complex; but would suggest this enables and applies a best of most worlds set of checks that will give community committers feedback on what might be improved 📈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

2 participants