search

tweag / cooked-validators

MIT License
39 stars 11 forks source link

Remove deprecated hooks and developpers doc #247

Closed florentc closed 1 year ago

florentc commented 1 year ago

This removes the legacy ormolu pre-commit hook that has now been replaced by hooks declared in the flake. It also removes the associated paragraph from the README.

This PR could also serve to clean up other devops/CI related deprecated stuff you would think of.

This also fixes #188

gabrielhdt commented 1 year ago

Don´t you want to even remove the paragraph "Developer Tools and Environment"? Ultimately we should probably aim for a "Getting started" section where the basic things should be mentioned. More advanced configuration should go to the hypothetical HACKING file.

gabrielhdt commented 1 year ago

You can also remove the file ci/commit-and-push-cabal-changes.sh, it's not used anywhere.

Niols commented 1 year ago

I would almost want to get rid of the whole ci/ directory, because it mostly contains ad-hoc stuff. But some things in there are still useful, in particular the run-tests.sh script. But I do think it could be inlined easily in the CI workflow, which would even bring more readability.

florentc commented 1 year ago

I removed the dev section from the README and commit-and-push-cabal in ci.

What is the state of hlint.yaml?

I noticed that build-haddock has a shebang that may not play nice with nix: #!/bin/bash instead of #!/usr/bin/env bash.

On the nitpicking side, build-haddock has no .sh extension whereas run-tests.sh does.

This could all be left for a future PR.

Niols commented 1 year ago

What is the state of hlint.yaml?

I'm not sure but I think we could just get rid about it.

I noticed that build-haddock has a shebang that may not play nice with nix: #!/bin/bash instead of #!/usr/bin/env bash.

On the nitpicking side, build-haddock has no .sh extension whereas run-tests.sh does.

This could all be left for a future PR.

Personally, I would vote for inlining both build-haddock and run-tests.sh in the corresponding CI workflows, the only places where they are used. This would allow us to get rid of this directory entirely. This could be done in this PR or a subsequent one.

gabrielhdt commented 1 year ago

For the hlint, I did not even know it existed, perhaps @Niols has some idea. IMO you can merge right away, or implement these nitpicks and merge. Consistency improvements are always welcome.