search

kevin-lee / refined4s

newtype and refinement (refined) type for Scala 3
https://refined4s.kevinly.dev
MIT License
10 stars 2 forks source link

Add KeyDecoder/KeyEncoder for circe #353

Closed ivan-klass closed 2 months ago

kevin-lee commented 2 months ago

Thank you, @ivan-klass, for the PR. Unfortunately, it requires more changes than what you've made. I'll get back to you on this PR after I address the other parts.

ivan-klass commented 2 months ago

@kevin-lee could you please list the missing things? May be I can follow up

kevin-lee commented 2 months ago

@ivan-klass Sure,

  1. Please remove all the test code you added. That's not the correct place to put the test for auto.
  2. This is the right place to add the tests. So you need to add tests for KeyEncoder and KeyDecoder to CustomTypeSpec.scala, networkSpec.scala, numericSpec.scala, and stringsSpec.scala.
  3. Unfortunately, your current code doesn't compile, please make sure the code compiles.

If you feel that adding tests to all those types is too much and time-consuming, and if you want to have the new auto with KeyEncoder and KeyDecoder released sooner, just let me know. You can keep the code without tests, and I can sort out the tests for a faster release.

kevin-lee commented 2 months ago

Also, please don't forget to rebase the current origin/main.

kevin-lee commented 2 months ago

@ivan-klass Sorry, I had to release a new version to use the KeyEncoder and KeyDecoder, including the auto one. If you're interested, here's how I did it. https://github.com/kevin-lee/refined4s/pull/371/files

Also all the other KeyEncoders and KeyDecoders.

ivan-klass commented 2 months ago

@kevin-lee thank you very much!