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

Importing Database.Esqueleto confusing in 4.0.0.0 #368

Open
m4dc4p opened this issue Jul 31, 2023 · 4 comments
Open

Importing Database.Esqueleto confusing in 4.0.0.0 #368

m4dc4p opened this issue Jul 31, 2023 · 4 comments

Comments

@m4dc4p
Copy link
Contributor

m4dc4p commented Jul 31, 2023

4.0.0.0 (esqueleto-next/ #325) updates Database.Esqueleto to the new syntax. I was migrating some code and struggled with confusing error messages ("Could not deduce (Database.Esqueleto.Internal.Internal.SqlSelect a0 (Entity record)) arising from a use of ‘select’") for some time before realizing I just needed to change Database.Esqueleto to Database.Esqueleto.Legacy.

I know the deprecation warning has been on Experimental forever, and possibly on Database.Esqueleto, but even so this caused me a good hour of frustration.

Is there some way to highlight the change? A migration guide or deprecation warning?

Thanks for all your excellent work!

@parsonsmatt
Copy link
Collaborator

There is a warning on Database.Esqueleto since version 3.5.0.0 which was uploadedin May of 2021, and has been on Stackage LTS since 18.1, released July 5th 2021.

I'm curious what your workflow is that you missed this warning.

@m4dc4p
Copy link
Contributor Author

m4dc4p commented Jul 31, 2023

I work on a large application with hundreds of modules and a lot of dependencies. We use nix for building in CI and our dev environment. The warning became noise in all that log spam. Additionally, the change needed to happen in dependent libraries, so I was trying to patch a dependency rather than our own code, so it just wasn't immediately obvious to me that I needed to update the import.

A warning at time-of-breakage (that is, on this release) would have saved me an hour or so.

@parsonsmatt
Copy link
Collaborator

esqueleto-4 has not been released yet - you're on a pre-release git branch, so that does give us some time to improve this experience.

Still, I'm not exactly sure what you're proposing or expecting. Do you want for esqueleto-4 to have a Database.Esqueleto that with a warning like This module exports the new syntax. If you still need legacy syntax, import Database.Esqueleto.Legacy, but beware that it will be deleted in an upcoming major version. ?

I'd like for esqueleto-4 to be the "import Database.Esqueleto to use new syntax without any warnings." Do you suppose that an esqueleto-3.6 which does the switch + warn would have helped? I don't think so, since you jumped to esqueleto-4 instead of "most recent Hackage release," but I'm happy to hear your thoughts.

@m4dc4p
Copy link
Contributor Author

m4dc4p commented Jul 31, 2023

Do you want for esqueleto-4 to have a Database.Esqueleto that with a warning like [...]

That pretty much would have avoided the problem for me.

Do you suppose that an esqueleto-3.6 which does the switch + warn would have helped?

I picked up this branch as I wanted to try the new window function support, so in that sense I'm an outlier - I definitely chose the pre-release branch. Its pretty hard to say if a 3.6 would help others or not. FWIW, I ran into this problem when recompiling persistent-pagination. Considering how that library is impacted by someone upgrading esqueleto might help clarify what you want to do.

Appreciate the discussion and hope this helps!

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

No branches or pull requests

2 participants