Skip to content

Allow rows in instance contexts#863

Merged
paf31 merged 5 commits intomasterfrom
642
Feb 11, 2015
Merged

Allow rows in instance contexts#863
paf31 merged 5 commits intomasterfrom
642

Conversation

@paf31
Copy link
Copy Markdown
Contributor

@paf31 paf31 commented Feb 11, 2015

@apsk @garyb Could you please review?

@apsk I worked from your pull request, but I needed to make some changes to support some additional use cases, and also added a couple of comments to explain what's going on and what needs to be improved.

Resolves #642.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.17%) to 83.31% when pulling 5d2bf0b on 642 into 7b5cb4b on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.15%) to 83.33% when pulling c9932ea on 642 into 7b5cb4b on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.13%) to 83.35% when pulling c9932ea on 642 into 7b5cb4b on master.

@garyb
Copy link
Copy Markdown
Member

garyb commented Feb 11, 2015

Awesome! 👍 👍 👍

It all looks great to me too, but I probably don't have a deep enough understanding of exactly what's going on to see any potential problems here anyway.

paf31 added a commit that referenced this pull request Feb 11, 2015
Allow rows in instance contexts
@paf31 paf31 merged commit d683d32 into master Feb 11, 2015
@paf31 paf31 deleted the 642 branch February 11, 2015 20:18
@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Feb 11, 2015

This will probably need more testing, but test-everything works for me. I'll make a release soon, and we can see if there are any issues.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.13%) to 83.35% when pulling 55388ba on 642 into 7b5cb4b on master.

@pseudonom
Copy link
Copy Markdown
Contributor

This is separate from allowing anonymous records? Like

instance showFoo :: Show ({foo :: String}) where
  show f = "f"

?

@garyb
Copy link
Copy Markdown
Member

garyb commented Mar 7, 2015

Yeah, that's what @paf31 meant in #863 when he mentioned disallowing rows in instance heads.

Closed rows like you have in that example would perhaps be workable, but maybe even that is problematic. @paf31?

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.

Allow rows in instance contexts

4 participants