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

[WPB-8712] use treefmt for everything #4000

Merged
merged 6 commits into from
Apr 22, 2024
Merged

[WPB-8712] use treefmt for everything #4000

merged 6 commits into from
Apr 22, 2024

Conversation

MangoIV
Copy link
Contributor

@MangoIV MangoIV commented Apr 15, 2024

https://wearezeta.atlassian.net/browse/WPB-8712

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Apr 15, 2024
@MangoIV
Copy link
Contributor Author

MangoIV commented Apr 15, 2024

I think I want to readd some make hlint-apply rule or somth

@MangoIV MangoIV marked this pull request as ready for review April 15, 2024 13:29
@MangoIV MangoIV changed the title [chore] use treefmt for everything [WPB-8712] use treefmt for everything Apr 15, 2024
@MangoIV MangoIV requested review from akshaymankar and removed request for akshaymankar April 16, 2024 14:34
@MangoIV
Copy link
Contributor Author

MangoIV commented Apr 17, 2024

there's one tradeoff to adding hlint to this is that it will make the initial lint very slow (will take ~5 minutes) but after then the cache kicks in and only changed files are re-linted.

changelog.d/5-internal/WPB-8712 Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
make hlint-inplace-pr
make hlint-check-pr # sometimes inplace has been observed not to do its job very well.
make format
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
make format
make format
make formatc # (at some point in the past, this has caught ormolu screwing up with inplace)

this might be fixed now with the --check-inplace argument, but can we be certain?

but if you prefer, we can remove this and see if it happens again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--check-idempotence? i have removed formatc, i think...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was hoping that ormolu by now is idempotent or at least the idempotency check works reliably 👀

Co-authored-by: Matthias Fischmann <mf@zerobuzz.net>
@MangoIV MangoIV merged commit 2d5073e into develop Apr 22, 2024
8 checks passed
@MangoIV MangoIV deleted the mangoiv/treefmt branch April 22, 2024 10:58
pcapriotti pushed a commit that referenced this pull request Apr 23, 2024
* [chore] use treefmt for everything

Co-authored-by: Matthias Fischmann <mf@zerobuzz.net>

---------

Co-authored-by: Matthias Fischmann <mf@zerobuzz.net>
MangoIV added a commit that referenced this pull request Apr 29, 2024
fisx pushed a commit that referenced this pull request Apr 30, 2024
* Revert "[chore] don't use treeefmt for hlint, readd the remove hlint rules (#4028)"

This reverts commit aef2f57.

* Revert "[WPB-8712] use treefmt for everything (#4000)"

This reverts commit 2d5073e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants