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

Only pull in 'postgres_backend' instead of 'postgres' #37

Merged
merged 2 commits into from
May 26, 2023

Conversation

aumetra
Copy link
Contributor

@aumetra aumetra commented May 21, 2023

When used with diesel-async, this crate enables the postgres feature on diesel which causes it to pull in libpq-sys, while postgres_backend suffices for this crate

Copy link
Member

@weiznich weiznich left a comment

Choose a reason for hiding this comment

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

I'm generally ok with this change, but we cannot do that now or in that way. Removing this feature from the default configuration would be a breaking change and we do not plane to release a major diesel_full_text_search version anytime soon.

I would be open for the following alternative approach:

  • Add a with-diesel-postgres feature to diesel_full_text_search
  • Add that feature as default feature
  • Only enable postgres_backend unconditionally for the diesel dependency
  • Add a CI job to test without default features

@aumetra
Copy link
Contributor Author

aumetra commented May 22, 2023

Yeah, I forgot about the semver implications of this change.
I hope this is how you imagined it, the CI is now using cargo-hack to build a powerset of all features, just so testing in the future is easier (in case features get added).

@aumetra aumetra requested a review from weiznich May 22, 2023 10:19
@weiznich
Copy link
Member

👍 Thanks for the update. It looks good now.

@weiznich weiznich merged commit fa59985 into diesel-rs:master May 26, 2023
6 checks passed
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.

None yet

2 participants