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

chore: add resources and services to workspace #1017

Closed

Conversation

jonaro00
Copy link
Member

@jonaro00 jonaro00 commented Jun 19, 2023

Description of change

  • add resources and services to workspace, and bump many minimum versions (semver compatible)
  • cargo update
  • added rustls feature flags to aws-rds
  • Bumped rocket rc2 -> rc3

How has this been tested? (if applicable)

@jonaro00 jonaro00 marked this pull request as ready for review June 19, 2023 22:05
@jonaro00
Copy link
Member Author

aws-rds and shared-db were left excluded from the workspace, as the --all-features tests would break on them due to sqlx feature flags.
I adjusted CI accordingly.

resources/shared-db/Cargo.toml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@jonaro00 jonaro00 force-pushed the chore/workspace-everything branch 2 times, most recently from ac77b37 to a05f064 Compare June 26, 2023 22:50
@jonaro00
Copy link
Member Author

jonaro00 commented Jul 3, 2023

Turso is also excluded, a comment in the root toml explains why.

@jonaro00
Copy link
Member Author

jonaro00 commented Jul 5, 2023

SQLX 0.7 is released, which should solve the turso thing, but that is a bigger job for a different PR.

Cargo.toml Outdated
shuttle-codegen = { path = "codegen" }
shuttle-common = { path = "common" }
shuttle-proto = { path = "proto" }
shuttle-service = { path = "service" }
Copy link
Member Author

Choose a reason for hiding this comment

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

@oddgrd Do you think removing the versions here will break something in the release process? I'm down to just "if it ain't broke, don't fix it" and add them back.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think we would need the versions to be able to publish the crates that use these. You should be able to test this with cargo publish --dry-run in one of the crates.

@jonaro00
Copy link
Member Author

When I tried to cargo update on this branch now, I got this cyclic dependency

error: cyclic package dependency: package `ahash v0.8.3` depends on itself. Cycle:
package `ahash v0.8.3`
    ... which satisfies dependency `ahash = "^0.8.0"` of package `hashbrown v0.14.0`
    ... which satisfies dependency `hashbrown = "^0.14"` of package `indexmap v2.0.0`
    ... which satisfies dependency `indexmap = "^2"` of package `serde_json v1.0.104`
    ... which satisfies dependency `serde_json = "^1.0"` of package `wasm-bindgen v0.2.87`
    ... which satisfies dependency `wasm-bindgen = "^0.2.87"` of package `js-sys v0.3.64`
    ... which satisfies dependency `js-sys = "^0.3"` of package `getrandom v0.2.10`
    ... which satisfies dependency `getrandom = "^0.2.0"` of package `const-random-macro v0.1.15`
    ... which satisfies dependency `const-random-macro = "^0.1.15"` of package `const-random v0.1.15`
    ... which satisfies dependency `const-random = "^0.1.12"` of package `ahash v0.8.3`
    ... which satisfies dependency `ahash = "^0.8"` of package `actix-http v3.3.1`

...which might not be our issue, but these kinds of things increase in probability if we jam more and more packages into the workspace 😕. I think we should be careful about the decision to merge this.

@oddgrd
Copy link
Contributor

oddgrd commented Jul 28, 2023

Yes, we have stumbled upon this hidden treasure in the rust ecosystem before. 😅 I agree that we should be careful merging this, since the advantage of doing so is not that significant either (correct me if I'm wrong). The PR has also grown quite large, perhaps we should rather split it up, merge the easy wins, and then re-consider how we will handle services and resources in the workspace in the future?

@jonaro00
Copy link
Member Author

The only easy wins I see are small CI improvements and the extended feature flags on aws-rds. Can be done but not very significant.

@jonaro00
Copy link
Member Author

#1167

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.

3 participants