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

Dependency pruning #247

Closed
wants to merge 3 commits into from
Closed

Dependency pruning #247

wants to merge 3 commits into from

Conversation

ayushjain17
Copy link
Collaborator

@ayushjain17 ayushjain17 commented Sep 24, 2024

Problem

  1. make run compiles code twice
  2. Redundant dependencies across all crates
  3. Single use dependency specified at workspace Cargo.toml
  4. superposition_types uses diesel as dependency, which causes dependency issues in 3rd party repos using this crate
  5. dependencies in random order
  6. cac_client returning data of types superposition_types::result::Result<T> in cases where the return should neither be a Result type or can simply be a Result<T, String>

Solution

  1. Remove redundant compile step
  2. Remove unused dependencies, mandate #![deny(unused_crate_dependencies)] for all crates
  3. Localise single use dependency to the crates instead of workspace Cargo.toml
  4. make result type as a feature for superposition_types (make diesel optional)
  5. sorted dependencies in alphabetical order
  6. Update return type of cac_client functions to direct T or Result<T, String>

@Datron
Copy link
Collaborator

Datron commented Sep 27, 2024

@ayushjain17 can we separate out dependency pruning and tenant config? That would make this PR way easier to review

@ayushjain17
Copy link
Collaborator Author

review

@Datron this PR is only for dependency pruning only, tenant config is just a pre-commit

If needed, you can review this PR, i will get the commit separated

@ayushjain17 ayushjain17 changed the base branch from tenantConfig to main September 27, 2024 10:43
@ayushjain17 ayushjain17 deleted the dependencyPruning branch September 27, 2024 11:01
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