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 FunctorEmpty and TraverseEmpty #51

Merged
merged 3 commits into from
Sep 6, 2018

Conversation

stephen-lazaro
Copy link
Contributor

@stephen-lazaro stephen-lazaro commented Aug 21, 2018

Resolves issue #47
cf. https://github.com/typelevel/cats/pull/2405/files
One instance cannot be discovered in the RWSTT tests, currently looking into that... Might need to wait on cats PR2405?

Issue typelevel#47
cf. https://github.com/typelevel/cats/pull/2405/files
One instance cannot be discovered in the RWSTT tests,
currently looking into that...
checkAll("ReaderWriterStateT[Option, Boolean, Int, String, ?]",
MonadLayerControlTests[ReaderWriterStateT[Option, Boolean, Int, String, ?], Option, (Int, String, ?)]
.monadLayerControl[Boolean, Boolean])
checkAll("MonadLayerControl[ReaderWriterStateT[Option, Boolean, Int, String, ?], Option]",
SerializableTests.serializable(MonadLayerControl[ReaderWriterStateT[Option, Boolean, Int, String, ?], Option]))

*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotta figure out why this instance is no longer found.

@codecov-io
Copy link

codecov-io commented Aug 21, 2018

Codecov Report

Merging #51 into master will decrease coverage by 1.58%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage   94.68%   93.09%   -1.59%     
==========================================
  Files          71       63       -8     
  Lines         734      565     -169     
  Branches        5        3       -2     
==========================================
- Hits          695      526     -169     
  Misses         39       39
Impacted Files Coverage Δ
.../main/scala/cats/mtl/hierarchy/BaseHierarchy.scala 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbc7a59...1cf0f66. Read the comment docs.

@LukaJCB
Copy link
Member

LukaJCB commented Aug 24, 2018

Thanks for this :) Looks good so far (though that instance not being found is odd)

@@ -9,6 +9,5 @@ trait AllInstances extends EitherTInstances
with RaiseInstances with ReaderTInstances
with LocalInstances with StateInstances
with StateTInstances with WriterTInstances
with EmptyInstances with ReaderWriterStateTInstances
Copy link
Member

Choose a reason for hiding this comment

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

Oh actually it's not odd, you deleted them here 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh my God 🤦‍♂️
Amazing. Thanks Luka, will fix.

Copy link
Member

Choose a reason for hiding this comment

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

No worries :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, fixed now!

Adds back accidentally deleted instances
@stephen-lazaro
Copy link
Contributor Author

Looks like code coverage went down due to the deletion of some hierarchy stuff that referenced FunctorEmpty. Is that acceptable or should I wait on PR2405 and then add it back?

@LukaJCB
Copy link
Member

LukaJCB commented Aug 28, 2018

I don't think we need to add that back, this should be fine :)

@stephen-lazaro
Copy link
Contributor Author

Cool, sounds good to me! Thanks Luka :)

@LukaJCB LukaJCB merged commit a5105e6 into typelevel:master Sep 6, 2018
@LukaJCB LukaJCB added this to the 0.4.0 milestone Sep 6, 2018
@stephen-lazaro stephen-lazaro deleted the remove_empty_classes branch September 6, 2018 17: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