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

Set the column-limit in fourmolu config #4175

Merged
merged 10 commits into from
Mar 18, 2024

Conversation

Lucsanszky
Copy link
Contributor

@Lucsanszky Lucsanszky commented Mar 6, 2024

Description

In addition to applying the formatting changes enforced by the added column-limit rule, this PR will also:

  • Remove the idempotence check when running fourmolize.sh as that doesn't play well with column-limit.
    See: https://fourmolu.github.io/config/column-limit/
  • Add .git-blame-ignore-revs file so we can keep track of commits with large formatting changes and ignore them when blaming.
  • Make it possible to run fourmolize.sh on changed files only by supplying the --changes flag.
  • Run the fourmolu GitHub action on changed files only (compared to origin/master).
  • Setup pre-commit in a non-intrusive way.

For now, I disabled running the pre-commit-check as a Hydra job until we decide whether to keep the current setup (running fourmolize.sh as a GitHub action) or switch to pre-commit. Both options have their advantages, however if at one point we decide to adopt iogx, I would advocate for pre-commit via Hydra. Regardless, pre-commit will be available from now on as a git hook and one can opt-in to use it conveniently, albeit in a bit hacky way (see the SKIP flag trick detailed in CONTRIBUTING.md).

Resolves #4069

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • When applicable, versions are updated in .cabal and CHANGELOG.md files according to the
    versioning process.
  • The version bounds in .cabal files for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • All visible changes are prepended to the latest section of a CHANGELOG.md for the affected packages. New section is never added with the code changes. (See RELEASING.md)
  • Code is formatted with fourmolu (use scripts/fourmolize.sh)
  • Cabal files are formatted (use scripts/cabal-format.sh)
  • hie.yaml has been updated (use scripts/gen-hie.sh)
  • Self-reviewed the diff

@Lucsanszky Lucsanszky force-pushed the lucsanszky/fourmolu-column-limit branch 17 times, most recently from 3ec3f18 to 061fd5e Compare March 13, 2024 03:26
@Lucsanszky Lucsanszky force-pushed the lucsanszky/fourmolu-column-limit branch 6 times, most recently from 509e1d3 to 8d477a1 Compare March 15, 2024 03:19
Copy link
Contributor Author

@Lucsanszky Lucsanszky left a comment

Choose a reason for hiding this comment

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

Well... There certainly are some interesting formatting changes but I think they are more or less fine. Except for one instance that I pointed out in the code, that one really is an eyesore.

@Lucsanszky Lucsanszky marked this pull request as ready for review March 15, 2024 05:55
scripts/fourmolize.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@teodanciu teodanciu left a comment

Choose a reason for hiding this comment

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

Looks good to me, very generous explanations!
I'm personally not going to use the pre-commit hook but I will certainly use the --changes flag with fourmolu, it will be great to not have to wait so long for the script.

The added `.git-blame-ignore-revs` file can be used with `git blame`
to ignore certain commits when blaming, which is useful
when we want to commit large formatting changes without messing up
the output of `git blame`. Also, by adding this file to the repo
`GitHub` will pick it up automatically and ignore the listed commits.

Usage:
`git blame --ignore-revs-file .git-blame-ignore-revs somefile.ext`
However, it is highly recommended that you configure `git blame`
locally to always ignore the commits specified in
`.git-blame-ignore-revs`. You can do this by running the following
from the repository's root:
`git config blame.ignoreRevsFile .git-blame-ignore-revs`.
From then on, you can just run `git blame somefile.ext` and ignoring
will also work for git porcelains and integrations automatically.
@Lucsanszky Lucsanszky force-pushed the lucsanszky/fourmolu-column-limit branch from 10e1571 to 40059da Compare March 17, 2024 20:31
This will make it possible to only run `fourmolu` on changes.
When the `--changes` flag is omitted, we default to the old
behaviour.
* Explain `pre-commit` setup
* Provide some examples
We both run formatting checks as a Hydra job via `pre-commit-check`
and as a GitHub action with `fourmolize.sh`. Disable the former for
now until we decide settling for one of them.
@Lucsanszky Lucsanszky force-pushed the lucsanszky/fourmolu-column-limit branch from 40059da to bf735c3 Compare March 17, 2024 20:38
@Lucsanszky Lucsanszky merged commit 387df93 into master Mar 18, 2024
15 checks passed
@neilmayhew neilmayhew deleted the lucsanszky/fourmolu-column-limit branch March 28, 2024 02:21
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.

Set the column-limit in fourmolu
3 participants