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

Remove NoImplicitPrelude from cardano-api #4832

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Jan 23, 2023

We import Prelude explicitly from the majority of cardano-api modules. Removing this language extension makes this unnecessary and removes confusion about the status of cardano-prelude.

@newhoggy newhoggy force-pushed the newhoggy/remove-NoImplicitPrelude-from-cardano-api branch from b84c973 to 18416b5 Compare January 23, 2023 19:30
@newhoggy newhoggy changed the title Remove NoImplicitPrelude from cardano-api. We import Prelude explici… Remove NoImplicitPrelude from cardano-api Jan 23, 2023
@newhoggy newhoggy marked this pull request as ready for review January 23, 2023 19:34
@newhoggy newhoggy force-pushed the newhoggy/remove-NoImplicitPrelude-from-cardano-api branch from 18416b5 to 2129f33 Compare January 24, 2023 16:20
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM! We should remove cardano-prelude in a follow up PR at some point

@@ -97,7 +96,6 @@ genCostModels = do
Right alonzoCostModel ->
Alonzo.CostModels . conv <$> Gen.list (Range.linear 1 3) (return alonzoCostModel)
where
conv :: [Alonzo.CostModel] -> Map Alonzo.Language Alonzo.CostModel
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the removal of the type signature here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason I got this error with the type signature:

gen/Gen/Cardano/Api.hs:99:33: error:
    • Expected kind ‘* -> *’,
        but ‘Map Alonzo.Language’ has kind ‘Metadatum’
    • In the type signature:
        conv :: [Alonzo.CostModel] -> Map Alonzo.Language Alonzo.CostModel
      In an equation for ‘genCostModels’:
          genCostModels
            = do CostModel cModel <- genCostModel
                 lang <- genLanguage
                 case Alonzo.mkCostModel lang cModel of
                   Left err -> panic . Text.pack $ "genCostModels: " <> show err
                   Right alonzoCostModel
                     -> Alonzo.CostModels . conv
                          <$> Gen.list (Range.linear 1 3) (return alonzoCostModel)
            where
                conv :: [Alonzo.CostModel] -> Map Alonzo.Language Alonzo.CostModel
                conv [] = mempty
                conv (c : rest)
                  = Map.singleton (Alonzo.getCostModelLanguage c) c <> conv rest
   |
99 |   conv :: [Alonzo.CostModel] -> Map Alonzo.Language Alonzo.CostModel
   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I need to qualify it:

  conv :: [Alonzo.CostModel] -> Map.Map Alonzo.Language Alonzo.CostModel

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 put the type signature back in.

@newhoggy newhoggy force-pushed the newhoggy/remove-NoImplicitPrelude-from-cardano-api branch from 2129f33 to 5eb9989 Compare January 24, 2023 17:12
@newhoggy newhoggy merged commit a5e4dd6 into master Jan 24, 2023
@newhoggy newhoggy deleted the newhoggy/remove-NoImplicitPrelude-from-cardano-api branch January 24, 2023 21:42
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