Skip to content

Conversation

@DougReeder
Copy link
Member

@DougReeder DougReeder commented Jun 26, 2025

What?

Adds setup script which enables a git pre-commit hook which pseudo-lints

Formats source files that weren't formatted according to the standard, using mix commit

Why?

The pseudo-lint is run by CI, so it should be run before committing locally.
There is no git-native way to check hooks into a project.
This does add a step to setup, but other setup actions can and should be moved to the setup script over time.

CI tests that files are formatted.

How to test

  1. create and switch to a throwaway branch
  2. run scripts/setup.sh
  3. edit an Elixir file that mix format doesn't currently complain about and add spaces to the end of a line
  4. attempt to commit the changed file; observe that commit is blocked w/ error message
  5. revert the changed file
  6. commit; observe that commit is not blocked

Documentation of functionality

README was updated with new step

Limitations

Doesn't run automated tests locally, as those are not currently working for me and we need first to understand whether the automated tests can be run on any developer machine.

Alternatives considered

Just configuring CI to fail unformatted files is a worse developer experience.

Additional details or related context

Also fails commit if any file contains non-ASCII characters. This is a new test, but existing files pass it.

Files from a number of commits were not formatted, as that wasn't enforced before.
The second commit here formats them.

This configures CI to fail lint-and-test if any file is not formatted.

If we can figure out what's going on, we should also run the automated tests before a commit.

Why: The pseudo-lint is run by CI, so it should be run before committing locally.
There is no git-native way to check hooks into a project.
This does add a set to setup, but other setup actions can and should be moved to the setup script over time.
Why: CI tests that files are formatted so.
@DougReeder
Copy link
Member Author

The text-matrix failures are the same as before — something needs to be fixed before we can use newer versions of Elixir and OTP

mix format on my Mac using Elixir 1.18.4 and Erlang 28.0.1 is fine with the line mix format is complaining about here.

FWIW, changing the line as this mix format wants still passes the automated tests: https://github.com/DougReeder/reticulum/actions/runs/15907766613/job/44867092914

Why: The format for fread should be a string: https://www.erlang.org/doc/apps/stdlib/io.html#fread/2
It will accept a charlist, but some versions of `mix format` flag that as an error.
…ixir and OTP

Why: This may detect some changes in Elixir that our old code must change to match.
@DougReeder DougReeder force-pushed the pre-commit-lint branch 2 times, most recently from 5c76298 to 3208e42 Compare September 9, 2025 20:02
Why: A commit shouldn't have to fix problems unrelated to its main purpose.
@Exairnous
Copy link
Member

LGTM! Thanks. Merging.

Copy link
Member

@Exairnous Exairnous left a comment

Choose a reason for hiding this comment

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

Add GitHub approval to make it happy.

@Exairnous Exairnous merged commit 183bc4f into master Sep 9, 2025
6 of 18 checks passed
@Exairnous Exairnous deleted the pre-commit-lint branch September 9, 2025 20:26
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.

3 participants