csv-validator
csv-validator copied to clipboard
digital-preservation
Remove scalaz from dependencies, replacing with cats implementations
cats-core and scalaz contain analogous data-structures and concepts (for our purposes they both implement NEL, and cats Validated is fundamentally the same as scalaz Validation).
Personally I find cats to be more ergonomic (for example the nel.list.toList code in scalaz that is no longer needed) and anecdotally it would appear cats is more widely adopted amongst the scala community, but the primary motivation for this change is that cats is a transitive dependency via fs2 now anyway (fs2 is built upon cats-effect), so whereas cats needs to be there scalaz is only now needed to provide things that cats has anyway.
Removing scalaz will therefore lead to a smaller uberjar, less libraries that need periodically upgrading, and a smaller stack of technologies that a new contributor would need to be familiar with.
Additionally I believe this should lend itself to a more consistent code style if moving to a fs2-based streaming approach more broadly (to me using the two together would be similar to using both metric and imperial measurements in the same technical drawing).
It should also be noted that there is a library for fs2/scalaz interop (https://github.com/functional-streams-for-scala/fs2-scalaz), but it's dead. If scalaz is definitely preferred over cats then we might want to look at replacing usage of fs2 with ZIO.
As an aside I'm not sure Validated[Nel[FailMessage], Any] is the best pattern here - I think the right hand side is typically discarded anyway, so might as well be Unit, which would make it bijective with List[FailMessage], with Nil being success (the absence of errors). This would probably make the interface much cleaner, but further changes (and testing) would be needed for this.
@luketebbs Hey Luke, this is a big change, can you give some description in the PR please as to the advantages you perceive from doing this?
@luketebbs Excellent. All good arguments. It makes a lot of sense to me :-)
It's up to date with master. I could merge in the CI changes too if you want, but that would make it harder to apply this but not the CI changes (should you prefer to do that). Alternatively I'm happy to resolve any conflicts between the second branch and master after the 1st is merged.
@luketebbs I think it would be best to get CI passing in main (via a separate PR) before we then rebase this
@luketebbs Now we have merged the build fix PR would you mind rebasing this, and then I can take a look