-
Notifications
You must be signed in to change notification settings - Fork 101
💥 Reorganize Crates #1034
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
💥 Reorganize Crates #1034
Conversation
02c9740 to
004d7b4
Compare
| @@ -1,10 +1,15 @@ | |||
| [package] | |||
| name = "temporal-sdk-core-c-bridge" | |||
| name = "temporalio-sdk-core-c-bridge" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, we should either call this temporalio-core-c-bridge or change the dir to sdk-core-c-bridge
| @@ -1,10 +1,7 @@ | |||
| [package] | |||
| name = "temporal-sdk-core" | |||
| name = "temporalio-sdk-core" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, we should either call this temporalio-core or change the dir to sdk-core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I can't decide if these should have sdk-core or just core in the name (in general). Open to suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can see arguments both ways. I like core-<whatever> because they aren't specific to SDK, but I like sdk-core-<whatever> because the less-wieldy name implies they shouldn't be directly depended on ever. Today it's the latter (so generated artifacts are like libtemporal_sdk_core_c_bridge.so) but we have no requirement to keep that name.
Also, I don't think the C bridge really has any extra dependencies (but it does run an extra build step), if you wanted to make it a module instead of a crate. But also fine leaving as a crate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went with rename of dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about top-level folders for tests and sdk-core-protos, should they similarly be reorg'd? I figure non-crate stuff like etc and docker and such we can discuss separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are. Tests is moved inside the core crate. Protos is inside common. Docker is moved inside etc with all other misc. stuff
004d7b4 to
efff346
Compare
efff346 to
8c2fd16
Compare
In prep for Rust SDK, reorganizes and renames crates according to the new structure.
Lang layers will need to do some find-replaces on their
usestatements, but otherwise no impact.