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 missing instances to (:&) #291

Merged
merged 5 commits into from
Sep 30, 2021

Conversation

m4dc4p
Copy link
Contributor

@m4dc4p m4dc4p commented Sep 28, 2021

The (:&) operator has an instance for SqlSelect, but none
for ToAlias and ToAliasReference. Adding those for parity.

I'm not sure if the should be a patch or major version bump. Adding new instances can break existing code, but they are such small instances ... I guessed and bumped the patch number.

I was unable to get the testing environment working locally, so I was going to rely on CI to execute my tests. 🤞

Related to #290.

After submitting your PR:

  • Update the Changelog.md file with a link to your PR.
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts).

The (:&) operator has an instance for `SqlSelect`, but none
for `ToAlias` and `ToAliasReference`. Adding those for parity.
Copy link
Collaborator

@parsonsmatt parsonsmatt left a comment

Choose a reason for hiding this comment

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

Looks fantastic!

I think a new instance is a minor version bump, not patch.

Could you add a changelog entry?

Thanks!

esqueleto.cabal Outdated Show resolved Hide resolved
src/Database/Esqueleto/Experimental/From/Join.hs Outdated Show resolved Hide resolved
src/Database/Esqueleto/Experimental/From/Join.hs Outdated Show resolved Hide resolved
@m4dc4p
Copy link
Contributor Author

m4dc4p commented Sep 28, 2021

Updates made.

@parsonsmatt
Copy link
Collaborator

awesome, thank you!

@m4dc4p
Copy link
Contributor Author

m4dc4p commented Sep 28, 2021

Would you approve the workflow again so CI will run? Apparently this will have to happen each time I push :/

test/Common/Test.hs Outdated Show resolved Hide resolved
test/Common/Test.hs Outdated Show resolved Hide resolved
test/Common/Test.hs Show resolved Hide resolved
m4dc4p and others added 2 commits September 30, 2021 09:47
Co-authored-by: Matt Parsons <parsonsmatt@gmail.com>
Co-authored-by: Matt Parsons <parsonsmatt@gmail.com>
@parsonsmatt
Copy link
Collaborator

going to merge and fix locally, thanks for the contribution! 😄

@parsonsmatt parsonsmatt merged commit ed4e98f into bitemyapp:master Sep 30, 2021
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