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 the test on derive #335

Merged
merged 2 commits into from
Nov 9, 2021
Merged

Remove the test on derive #335

merged 2 commits into from
Nov 9, 2021

Conversation

pm47
Copy link
Contributor

@pm47 pm47 commented Sep 7, 2021

There seems to have been a copy-paste mistake when rewriting those tests.

derive is awesome by the way!

There seems to have been a copy-paste mistake when rewriting those tests.

`derive` is awesome by the way!
@pm47 pm47 marked this pull request as draft September 7, 2021 14:57
@mpilquist
Copy link
Contributor

Thanks! I think we removed derive in 2.0 due to difficulties in getting it working with Scala 3 and the fact that consume was usually just as nice. We could try adding it back if you think it's worth it.

@pm47
Copy link
Contributor Author

pm47 commented Sep 7, 2021

we removed derive

So that explains why the build wasn't passing 😅

We could try adding it back if you think it's worth it.

For my use case consume works just as well, so you should probably not bother. I found derive particularly elegant though, it makes the intent more obvious imo.

I don't think I ever had the opportunity to thank you for scodec. It's a terrific library, we have been using it for years here https://github.com/ACINQ/eclair and are extremely happy about it.

The `derive` method has been removed.
@pm47 pm47 marked this pull request as ready for review September 7, 2021 17:12
@pm47 pm47 changed the title Fix regression in the derive example Remove the test on derive Sep 7, 2021
@mpilquist
Copy link
Contributor

@pm47 Thanks for the kind words! I'm glad scodec has been useful. Let me know if you have any suggestions!

@mpilquist mpilquist merged commit bed9754 into scodec:main Nov 9, 2021
@pm47 pm47 deleted the patch-1 branch November 9, 2021 12: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.

None yet

2 participants