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

Pull out a configuration library. #289

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

SamirTalwar
Copy link
Contributor

What

We require a configuration library so that we can re-use configuration types and functions in both a CLI plugin and a server-side connector build service.

This has been done with minimal effort.

Further work is required to minimize its surface area. Specifically, I believe we should:

  1. reduce the number of crates depending on this one,
  2. remove the dependency from "ndc-postgres-configuration" on "query-engine-sql" (it's only used to pull in IsolationLevel), and
  3. refactor to reduce the number of exposed types and functions as much as possible.

I have not done any of these things in this changeset to make it easier to review, and because I didn't want reviews of this extraction to get bogged down in whether we should do the above or not.

How

I moved ndc-postgres/src/configuration.rs and everything in ndc-postgres/src/configuration to their own crate, imaginatively named "ndc-postgres-configuration".

I then re-exported what was necessary from lib.rs, and changed imports in several places.

This has been done with minimal effort.

Further work is required to minimize its surface area.
@SamirTalwar SamirTalwar requested a review from a team February 12, 2024 16:33
Copy link
Contributor

@plcplc plcplc left a comment

Choose a reason for hiding this comment

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

LGTM

@SamirTalwar SamirTalwar added this pull request to the merge queue Feb 12, 2024
Merged via the queue into main with commit 28393ef Feb 12, 2024
28 checks passed
@SamirTalwar SamirTalwar deleted the samirtalwar/configuration-library branch February 12, 2024 17:50
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