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

Add more standard rules and eliminate Seq views #9

Merged

Conversation

Jacoby6000
Copy link
Collaborator

@Jacoby6000 Jacoby6000 commented Sep 15, 2017

In this PR, I've created strict variants of all the base property rules, and I've removed the Seq view restrictions that were on some of them... Instead, they now depend on either Foldable, Sizeable, Monoid, or Indexable.

  • Sizeable is just a typeclass for things which have size.
  • Indexable was updated to be based on mapWithIndex rather than zipWithIndex. This lets us generalize Indexable, such that forall F[_]: Foldable, F is also Indexable. Additionally, Indexable will be able to be removed once we move to cats 1.0.0, since these things are built in to Traverse there.
  • Monoid is used for the non-empty rule, rather than the seq view that was there before. This means users can define their own things which may be empty.

Some consequences of this, are that Seq won't work for anything which depends on Foldable. Depending on cats' alleycats package should get you unlawful typeclasses for those, however.

@codecov
Copy link

codecov bot commented Sep 15, 2017

Codecov Report

Merging #9 into develop will increase coverage by 2.12%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop       #9      +/-   ##
===========================================
+ Coverage    79.47%   81.59%   +2.12%     
===========================================
  Files           10       13       +3     
  Lines          151      201      +50     
  Branches         0        2       +2     
===========================================
+ Hits           120      164      +44     
- Misses          31       37       +6
Impacted Files Coverage Δ
checklist/src/main/scala/checklist/Indexable.scala 100% <100%> (+33.33%) ⬆️
...list/src/main/scala/checklist/SizeableSyntax.scala 100% <100%> (ø)
checklist/src/main/scala/checklist/Sizeable.scala 100% <100%> (ø)
...ist/src/main/scala/checklist/IndexableSyntax.scala 66.66% <66.66%> (ø)
checklist/src/main/scala/checklist/Rule.scala 80.91% <80.85%> (-0.34%) ⬇️

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 879db26...910e602. Read the comment docs.

@Jacoby6000
Copy link
Collaborator Author

@davegurnell Hey dave, just checking to see if you saw this.

@davegurnell
Copy link
Owner

davegurnell commented Oct 1, 2017

I have now! It all looks good except for one thing, which I think is a bigger design problem with the library in general that needs to be sorted in the medium term. I'll merge this now anyway and raise an issue to discuss the design problem.

@davegurnell davegurnell merged commit 42a4627 into davegurnell:develop Oct 1, 2017
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.

2 participants