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

feat(#399): add ToJSON GYTxOutRefCbor instance #406

Merged

Conversation

jaredponn
Copy link

@jaredponn jaredponn commented Feb 1, 2025

Summary

This PR:

  • adds the ToJSON GYTxOutRefCbor instance for issue Missing ToJSON GYTxOutRefCbor instance #399
  • documents some of the issues with this instance (it's not an isomorphism, but in practise this hopefully isn't an issue)
  • property based testing of roundtrip tests of the form GYTxOutRefCbor --> JSON --> GYTxOutRefCbor === id

Type of Change

Please mark the relevant option(s) for your pull request:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code refactoring (improving code quality without changing its behavior)
  • Documentation update (adding or updating documentation related to the project)

Checklist

Please ensure that your pull request meets the following criteria:

  • I have read the Contributing Guide
  • My code follows the project's coding style and best practices
  • My code is appropriately commented and includes relevant documentation
  • I have added tests to cover my changes
  • All new and existing tests pass
  • I have updated the documentation, if necessary

Testing

Please describe the tests you have added or modified, and provide any additional context or instructions needed to run the tests.

It was included in the usual test suite atlas-tests, so no special instructions are needed to run it.

Additional Information

Running my tests locally on my computer succeeds with:

# the test runner was modified to only run my tests.
$ cabal run test:atlas-tests
atlas
  GYTxOutRefCbor
    Roundtrip GYTxOutRefCbor --> JSON --> GYTxOutRefCbor === id: OK
      +++ OK, passed 100 tests.

All 1 tests passed (0.01s)

Would be happy to provide any other information as needed :)

@jaredponn jaredponn requested a review from a team as a code owner February 1, 2025 03:00
@jaredponn jaredponn marked this pull request as draft February 1, 2025 03:00
@jaredponn jaredponn force-pushed the jaredponn/add-tojson-gytxoutrefcbor branch from 67355b6 to 8531886 Compare February 1, 2025 04:46
@jaredponn jaredponn marked this pull request as ready for review February 1, 2025 04:51
@jaredponn jaredponn force-pushed the jaredponn/add-tojson-gytxoutrefcbor branch from 8531886 to d1ed880 Compare February 1, 2025 04:52
@jaredponn jaredponn changed the title Add ToJSON GYTxOutRefCbor instance feat(#399): add ToJSON GYTxOutRefCbor instance Feb 1, 2025
Copy link
Contributor

@sourabhxyz sourabhxyz left a comment

Choose a reason for hiding this comment

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

Good job, thank you!

@4TT1L4 4TT1L4 merged commit 8906bf5 into geniusyield:main Feb 6, 2025
1 of 2 checks passed
@jaredponn
Copy link
Author

Amazing! Thank you @sourabhxyz for all the help and discussions with getting this merged, I really appreciate it!

@jaredponn jaredponn deleted the jaredponn/add-tojson-gytxoutrefcbor branch February 7, 2025 04:47
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