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

Replace askLogFunc with askLoggerIO #1162

Merged
merged 7 commits into from
Jan 6, 2021

Conversation

mitchellvitez
Copy link
Contributor

@mitchellvitez mitchellvitez commented Nov 22, 2020

Just trying to do a small FIXME here

no haddock changes

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)

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.

Thanks for the PR! I think it'd be best if we made an issue for this and tagged it in for the next major version bump. It doesn't seem like a large enough change to release all on it's own right now.

@@ -186,7 +186,7 @@ createPostgresqlPool = createPostgresqlPoolModified (const $ return ())
--
-- @since 2.1.3
createPostgresqlPoolModified
:: (MonadUnliftIO m, MonadLogger m)
:: (MonadUnliftIO m, MonadLoggerIO m)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing the signature of these functions is a breaking change and would require a major version bump.

@@ -2,6 +2,8 @@

## 2.11.0.1

* [#1162](https://github.com/yesodweb/persistent/pull/1162)
* Replace `askLogFunc` with `askLoggerIO`
Copy link
Collaborator

Choose a reason for hiding this comment

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

2.11.0.1 has already been released on Hackage - since this is a breaking change, it'd need to go under 2.12 or 3.0.

@mitchellvitez
Copy link
Contributor Author

best if we made an issue for this and tagged it in for the next major version bump. It doesn't seem like a large enough change to release all on it's own right now.

Agreed. To do that, do I just need to put this change under 2.12 in the changelog?

Also wasn't sure how to get CI building, looks like a github actions thing

@parsonsmatt
Copy link
Collaborator

I fixed CI on master, try merging that in.

I'm hesitant to release 2.12 with only this change, and I'd prefer to batch it up in the next breaking release.

@MaxGabriel MaxGabriel added this to the 2.12 milestone Dec 1, 2020
@parsonsmatt parsonsmatt merged commit f69716d into yesodweb:master Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants