Skip to content

Prepare for 2.0 release #18

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 9, 2016
Merged

Prepare for 2.0 release #18

merged 1 commit into from
Oct 9, 2016

Conversation

garyb
Copy link
Member

@garyb garyb commented Oct 8, 2016

No description provided.

natefaubion
natefaubion reviewed Oct 8, 2016
View reviewed changes
src/Data/Enum.purs

derive instance eqCardinality :: Eq (Cardinality a)

derive instance ordCardinality :: Eq (Cardinality a)

Copy link

Choose a reason for hiding this comment

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

You are deriving Eq twice :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, thanks!

paf31
paf31 reviewed Oct 8, 2016
View reviewed changes
src/Data/Enum.purs

instance boundedEnumTuple :: (BoundedEnum a, BoundedEnum b) => BoundedEnum (Tuple a b) where
cardinality =
Cardinality
$ runCardinality (cardinality :: Cardinality a)
* runCardinality (cardinality :: Cardinality b)
$ unwrap (cardinality :: Cardinality a)
Copy link
Contributor

@paf31 paf31 Oct 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe derive Semiring for Cardinality a?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did consider that... I guess it probably would be cleaner on balance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right, I did try it in fact, but I remember now: it doesn't help with almost any of these cases since we're combining Cardinality values with different phantom types

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

All good otherwise?

@paf31
Copy link
Contributor

paf31 commented Oct 9, 2016

Yes I think so

Sent from my iPhone

On Oct 9, 2016, at 1:54 PM, Gary Burgess notifications@github.com wrote:

@garyb commented on this pull request.

In src/Data/Enum.purs:

instance boundedEnumTuple :: (BoundedEnum a, BoundedEnum b) => BoundedEnum (Tuple a b) where
cardinality =
Cardinality

  •  $ runCardinality (cardinality :: Cardinality a)

  •  \* runCardinality (cardinality :: Cardinality b)

  •  $ unwrap (cardinality :: Cardinality a)
    All good otherwise?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@garyb garyb merged commit c3cd333 into master Oct 9, 2016
@garyb garyb deleted the bump branch October 9, 2016 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natefaubion natefaubion natefaubion left review comments

@paf31 paf31 paf31 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@garyb @paf31 @natefaubion