wheaties / AutoLifts

Automatic functional lifting, mapping and folding.
Apache License 2.0
65 stars 4 forks source link

Removed unused object #50

Closed Jentsch closed 8 years ago

Jentsch commented 8 years ago

The tests are still green. What is the purpose of those companion objects? Could I remove them? If yes I would amend this pull request with more commits.

codecov-io commented 8 years ago

Current coverage is 72.50%

Merging #50 into 0.5 will increase coverage by +11.18% as of 6e421c1

@@              0.5    #50   diff @@
====================================
  Files          57     57       
  Stmts         212    291    +79
  Branches        0      1     +1
  Methods         0      0       
====================================
+ Hit           130    211    +81
  Partial         0      0       
+ Missed         82     80     -2

Review entire Coverage Diff as of 6e421c1

Powered by Codecov. Updated on successful CI builds.

wheaties commented 8 years ago

Take a look at the export-hook project. That's where they come from. To be honest, we're not getting what was hoped we'd get out of it. The idea being that not only would it help solve orphan type classes (i.e. there wouldn't have to even be a ScalazLiftMap but just another object that implemented LiftMap in the autolift-scalaz project.) It's also not helping with reducing import overhead (a la reexports) due to the path dependent types. That could change if and when someone spent some time and effort to make that project more robust in regards to type members.

TL;DR If you want to take them out, might as well take out the export-hook dependency. We might even be able to lose the macro-paradise plugin as well.

Jentsch commented 8 years ago

I removed all references to the export-hook, macro-paradise and scala-reflect as well. Still all tests are green.

wheaties commented 8 years ago

Can't argue with this change. If we're not getting the benefits we want, there's no point in using it. I think it's definitely something that should be revisited in the future.