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

Upgrade to ormolu 0.1.2.0 #1145

Merged
merged 4 commits into from
Jul 31, 2020
Merged

Upgrade to ormolu 0.1.2.0 #1145

merged 4 commits into from
Jul 31, 2020

Conversation

mheinzel
Copy link
Contributor

@mheinzel mheinzel commented Jun 24, 2020

See https://github.com/zinfra/backend-issues/issues/1623

Don't merge yet. We'll wait for a good moment where we don't have many PRs with Haskell changes in progress.
It should not take much time to rebase this PR then (if we just re-create the run ormolu commit instead of rebasing it).

Main Ormolu changes since 0.0.5.0 we care about:

  • Blank lines between definitions in let and while bindings are now preserved. Issue 554.
  • Fixed rendering of comments around if expressions. Issue 458.
  • A bunch of idempotence fixes we occasionally ran into.

Changes that cause most diffs:

  • Improved sorting of operators in imports. Issue 602.
  • Added support for formatting linked lists with (:) as line terminator. Issue 478.
  • Renamed the --check-idempotency flag to --check-idempotence. (this needed fixing in our script as well)

@mheinzel mheinzel changed the title Upgrade to ormolu 0.1.1.0 and headroom 0.2.2.1 Upgrade to ormolu 0.1.2.0 Jul 27, 2020
@mheinzel mheinzel marked this pull request as ready for review July 27, 2020 17:41
@mheinzel
Copy link
Contributor Author

The failure of haskell-ormolu-ubuntu is expected since CI still has the old Ormolu version. It's nice that it detects that properly.

Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

This is nice!

Perhaps you want to wait with merging until we've merged #1180, and are ready to use it.

@mheinzel
Copy link
Contributor Author

Perhaps you want to wait with merging until we've merged #1180, and are ready to use it.

We could already use the script without it being merged. Might even make sense so we can battle-test it and adjust it as we go.

This decreases the diff resulting from running Ormolu 0.1.2.0 from
  226 files changed, 2611 insertions(+), 2316 deletions(-)
to
  213 files changed, 1968 insertions(+), 1759 deletions(-)
mheinzel added a commit that referenced this pull request Jul 31, 2020
Upgrading Ormolu can be painful, because the style will often change in
a lot of places (see for example #1145). Any active branches/PRs will
then have to rebase onto these changes and resolve the resulting
conflicts.
It becomes a bit less cumbersome after first squashing all commits of
the branch into a single one, but that's often not desirable.

This script will automate the rebasing process, keeping the (linear)
history of intact and making the commits appear as if the changes had
been applied onto the newly-formatted version all along.
@mheinzel
Copy link
Contributor Author

I'll merge this once the integration tests passed.

(for reference, the BASE_COMMIT to rebase onto these formatting changes will be 029ebd5458985bec3374247484d7d8703e2d4f0b)

@mheinzel mheinzel merged commit 086ebb3 into develop Jul 31, 2020
@mheinzel mheinzel deleted the mheinzel/upgrade-ormolu branch July 31, 2020 21:52
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.

2 participants