Skip to content

Upgrades "Ormolu" to a lot newer version "0.5.0.1" from older "0.1.4.1"#10

Draft
arjunkathuria wants to merge 1 commit intonammayatri:Mobility/GHC-9.2.7from
arjunkathuria:Mobilty/GHC-9.2.7
Draft

Upgrades "Ormolu" to a lot newer version "0.5.0.1" from older "0.1.4.1"#10
arjunkathuria wants to merge 1 commit intonammayatri:Mobility/GHC-9.2.7from
arjunkathuria:Mobilty/GHC-9.2.7

Conversation

@arjunkathuria
Copy link
Contributor

@arjunkathuria arjunkathuria commented May 16, 2023 •
edited
Loading

Attempts to solve nammayatri/nammayatri#737

Key changes made in this PR include:

  • Effectivly upgrades ormolu from "0.1.4.1" -> "0.5.0.1"

  • Deletes code that disabled Ormolu in HLS in "nix/haskell.nix" file

  • Removes the following, now redundant inputs from flake.nix file:

    • nixpkgs-21_11
    • nixpkgs-140774-workaround

    ^ They aren't required now that we use upstream "ormolu" version from nixpkgs-unstable.

  • Updates flake.lock file with the above mentioned changes incorporated.

@arjunkathuria
Upgrades "Ormolu" to a lot newer version "0.5.0.1" from older "0.1.4.1"
339b20f 
* Effectivly upgrades ormolu from "0.1.4.1" -> "0.5.0.1"

* Deletes code that disabled Ormolu in HLS in "nix/haskell.nix"

* Removes the following, now redundant inputs from flake.nix file:
  - nixpkgs-21_11
  - nixpkgs-140774-workaround

  ^ They aren't required now that we use upstream "ormolu" version

* Updates flake.lock file with the above mentioned changes incorporated.
@arjunkathuria arjunkathuria requested a review from srid May 16, 2023 10:52
@arjunkathuria arjunkathuria self-assigned this May 16, 2023
@arjunkathuria arjunkathuria marked this pull request as draft May 16, 2023 12:35
@arjunkathuria
Copy link
Contributor Author

arjunkathuria commented May 16, 2023

Seems ormolu dropped support for record-dot-preprocessor in version 0.4.0.0 (link from their changelog) and nudges towards using OverloadedRecordDot syntax instead.

This ends up adding a space between the getters a.b turns into a . b now, which would get triggered by the pre-commit hook and break code.

Another reason to look at nammayatri/nammayatri#895 .

@arjunkathuria arjunkathuria removed their assignment May 16, 2023
srid
srid reviewed May 16, 2023
View reviewed changes
Copy link
Member

@srid srid left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Diff itself looks good. But, before I merge I'd like to know ... do you intend to do nammayatri/nammayatri#737 as part of GHC 9.x upgrade on all 3 repos (nammayatri, shared-kernel, beckn-gateway)?

nix/treefmt.nix
# programs.hlint.enable = true;

# Ormolu Version is now at: 0.5.0.1
# TODO:// look into updating it to newest version 0.6.0.1
Copy link
Member

@srid srid May 16, 2023

Choose a reason for hiding this comment

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

Why is this TODO relevant?

@arjunkathuria
Copy link
Contributor Author

arjunkathuria commented May 16, 2023

But, before I merge I'd like to know ... do you intend to do nammayatri/nammayatri#737 as part of GHC 9.x upgrade on all 3 repos (nammayatri, shared-kernel, beckn-gateway)?

I was trying to see if i could. Turns out it's not that straight-forward. see the previous comment.

it does not support the record-dot-preprocessor plugin we tell it use here

(Drafted the PR, do not merge yet)

@arjunkathuria arjunkathuria mentioned this pull request Jun 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srid srid srid left review comments

Assignees

No one assigned

Labels

None yet

Projects

Namma-Yatri
Status: 👀 In review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@arjunkathuria @srid