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.

Sign up for GitHub

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

Jump to bottom

Add ToSchema instance for AssetClass (fix #3298) #3303

Merged
merged 3 commits into from Jun 4, 2021
Merged

Add ToSchema instance for AssetClass (fix #3298) #3303

merged 3 commits into from Jun 4, 2021

Conversation

ak3n
Copy link
Contributor

@ak3n ak3n commented Jun 3, 2021

Fix #3298

Pre-submit checklist:

  • Branch
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Pre-merge checklist:

  • Someone approved it
  • Commits have useful messages
  • Review clarifications made it into the code
  • History is moderately tidy; or going to squash-merge

@ak3n ak3n requested review from michaelpj and j-mueller June 3, 2021 08:25
silky
silky reviewed Jun 3, 2021
View reviewed changes
plutus-use-cases/src/Plutus/Contracts/Uniswap/Types.hs
@@ -52,9 +52,6 @@ data Liquidity = Liquidity
PlutusTx.makeIsDataIndexed ''Liquidity [('Liquidity, 0)]
PlutusTx.makeLift ''Liquidity

-- Note: An orphan instance here because of the newtype wrapper below.
deriving anyclass instance ToSchema AssetClass
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

i think you can then remove the ghc warning at the top of this file too (no-warn-orphans)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, thanks!

@michaelpj michaelpj merged commit 8868f4f into master Jun 4, 2021
@ak3n ak3n deleted the add-toschema-assetclass branch June 4, 2021 07:30
jakzale pushed a commit that referenced this pull request Jun 4, 2021
@ak3n
Add ToSchema instance for AssetClass (fix #3298) (#3303)
052ad03 
* Add ToSchema instance for AssetClass (fix #3298)

* Remove ToSchema's AssetClass instance in Uniswap use case

* Remove no-warn-orphans
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silky silky silky left review comments

@michaelpj michaelpj michaelpj approved these changes

@j-mueller j-mueller j-mueller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No ToSchema instance for AssetClass
4 participants
@ak3n @silky @michaelpj @j-mueller