Contributor

This is a PR for issue #869

Before submitting your PR, check that you've:

• Bumped the version number

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)

Contributor Author

The CI is failing because of something in MongoDB?

Collaborator

That's a really strange failure...

Contributor Author

All builds using stack work with no problem, but built with cabal, it breaks on a missing MonadFail constraint.

Collaborator

I suspect we're missing an upper bound somewhere...

Collaborator

Try merging master - there's an upper bound in place to prevent the cabal solver from picking that version.

Collaborator

Choose a reason for hiding this comment

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

Contributor Author

I'm trying to add some tests, but it's all really annoying.
Also, all of the methods from typeclasses have the backend type variable as number one, because that's the typeclass parameter. Don't really know if there'd be a way to make that more user friendly...

Contributor Author

I guess one way to do it, would be to not export the class methods in Database.Persist, and make new functions that have the same name so anyone importing Database.Persist will be able to use TypeApplications and anyone making their own (e.g. PersistStoreWrite) instance can use Database.Persist.Class

Then again, there are only a few functions that anyone might actually use with TypeApplications, right? The select* ones, and the delete* ones.

Collaborator

I wouldn't worry about tests for this. It's not like it can be broken.

You'd only really want to specify the rec where it could be ambiguous, like selectList and deleteWhere. I wouldn't worry too much about duplicating the names. eg insert can't be used polymorphically so it wouldn't make sense to specify the type.

Contributor Author

Well, I did anyway.

But yeah, selectList will have to still be used as selectList @_ @Table, since the backend parameter is first, because of the fact that it's a typeclass method. Same with deleteWhere @_ @Table. :/

Collaborator

I think selectList is fine since it's not a class method. selectFirst, count, selectSource are all affected though :\ PersistQueryWrite is also affected, annoying.

Well.

Hmm! Maybe it is worthwhile to have the class methods separate from the functions in the API. It's a breaking change, but all of this is a breaking change anyway.

Contributor Author

Ah true, selectList was outside of the typeclass.

Do you want me to add the select* and delete* functions in Database.Persist?

Collaborator

looking forward to merging this <3