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

Format with ormolu #549

Merged

Conversation

shayne-fletcher
Copy link
Contributor

prefer join over ++ and format with ormolu.

@shayne-fletcher shayne-fletcher force-pushed the format-with-ormolu branch 2 times, most recently from b067dae to 7d1fdaf Compare July 30, 2024 11:16
@shayne-fletcher shayne-fletcher marked this pull request as ready for review July 30, 2024 13:27
@shayne-fletcher shayne-fletcher requested a review from a team as a code owner July 30, 2024 13:27
@samuel-williams-da
Copy link
Contributor

Could we have this checked in CI/precommit of some kind?

@samuel-williams-da
Copy link
Contributor

samuel-williams-da commented Jul 30, 2024

Also, why the preference of join over ++ or <>, at least in cases with only 2 lists?

@shayne-fletcher
Copy link
Contributor Author

Could we have this checked in CI/precommit of some kind?

it sounds like a good idea! i'm not quite sure what you mean. could you elaborate?

@shayne-fletcher
Copy link
Contributor Author

Also, why the preference of join over ++ or <>, at least in cases with only 2 lists?

ah! the end goal was to format with ormolu but they way it rendered (longer lists) with ++ just looked so terrible (to me) so i resolved to use join before applying it. i'm not opposed to ++ in general. i think the convention for join is working well here; if you see ++ now it's in the interior of a list, not top-level. i just blanket replaced the ++s so didn't distinguish between short or long lists.

@shayne-fletcher
Copy link
Contributor Author

Could we have this checked in CI/precommit of some kind?

maybe you mean, a CI job that builds ghc-lib from the current GHC HEAD commit? we can do that. it's implemented as cabal run exe:ghc-lib-build-tool -- --ghc-flavor ""

@paulbrauner-da
Copy link

Could we have this checked in CI/precommit of some kind?

it sounds like a good idea! i'm not quite sure what you mean. could you elaborate?

Hi Shayne. I think what @samuel-williams-da means is we could have a pre-commit hook that runs the formatter, and we could have a CI tasks that fails if the code is not formatted.

@shayne-fletcher
Copy link
Contributor Author

Could we have this checked in CI/precommit of some kind?

it sounds like a good idea! i'm not quite sure what you mean. could you elaborate?

Hi Shayne. I think what @samuel-williams-da means is we could have a pre-commit hook that runs the formatter, and we could have a CI tasks that fails if the code is not formatted.

i see. yes, that would be good.

@samuel-williams-da
Copy link
Contributor

samuel-williams-da commented Jul 31, 2024

Paul is correct, it should be relatively easy to add as a job.
Regarding ++, I see ormolu does make it kind of ugly, I don't mind either way

@shayne-fletcher
Copy link
Contributor Author

Paul is correct, it should be relatively easy to add as a job.

yes, it's easy. similarly we should reintroduce hlint. can these things be done in a later PR though please?

@samuel-williams-da
Copy link
Contributor

Sure I'm okay with that

@samuel-williams-da samuel-williams-da merged commit b0493ab into digital-asset:master Jul 31, 2024
25 checks passed
@shayne-fletcher shayne-fletcher deleted the format-with-ormolu branch July 31, 2024 14:57
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