Combine sdk and cli #290
Combine sdk and cli #290
Conversation
There was a problem hiding this comment.
I understand that this is a straightforward conversion, but this is not how it should generally be done. First, the repository should be converted into workspace, with pulsar being not the root package, but rather one of workspace packages instead. Then SDK should be added as another workspace member that Pulsar can use, that way you have one lock file and don't need such a messy CI and cd sdk in strategic places.
| # Since, SDK and pulsar both uses async heavily, we need to disallow below type to prevent OS thread block | ||
| # This is imported from SDK | ||
| disallowed-types = ["std::sync::Mutex", "std::sync::RwLock", "std::sync::Once", "std::sync::Condvar"] |
There was a problem hiding this comment.
Honestly it doesn't make a lot of sense to me. There is nothing wrong with these types if they are use properly and clippy will already warn when potentially used incorrectly.
| anyhow = "1" | ||
| clap = { version = "4", features = ["derive"] } | ||
| derive_builder = "0.12" | ||
| derive_more = "0.99" | ||
| fdlimit = "0.2" | ||
| futures = "0.3" | ||
| serde_json = "1" | ||
| subspace-farmer-components = { git = "https://github.com/subspace/subspace", rev = "bd435100200b3dcce6d6f50534d52e3cd039ca8e" } | ||
| tempfile = "3" | ||
| tokio = { version = "1.34.0", features = ["rt-multi-thread", "macros"] } | ||
| tracing = "0.1" | ||
| tracing-futures = "0.2" | ||
| tracing-subscriber = "0.3" |
There was a problem hiding this comment.
Examples and integrations tests should be under sdk, not main package.
If we want to go with workspace route, we cannot have sdk as member, since it is itself a workspace and nested workspaces are not allowed (yet). In this case, we will need to put members of sdk workspace as part of the root workspace with examples and integration tests in root. |
Yes
As I explained above, we don't need anything at the root. The root should be just a workspace, see how monorepo is organized. |
|
I cancelled extra workflow for now to open CI time for other jobs, the manually triggered workflow is still running |
|
|
||
| jobs: | ||
| clippy-fmt-test: | ||
| fmt: | ||
| runs-on: ${{ fromJson(github.repository_owner == 'subspace' && '["self-hosted", "ubuntu-20.04-x86-64"]' || 'ubuntu-22.04') }} |
There was a problem hiding this comment.
| runs-on: ${{ fromJson(github.repository_owner == 'subspace' && '["self-hosted", "ubuntu-20.04-x86-64"]' || 'ubuntu-22.04') }} | |
| runs-on: ${{ fromJson(github.repository_owner == 'subspace' && '["self-hosted", "ubuntu-20.04-x86-64"]' || '"ubuntu-22.04"') }} |
|
Closing this in favor of #291 |
Description
This PR combines subspace-sdk into this repository.
What is changed?
First commit
First commit combines sdk and cli and some changes to make it compile and test successfully. The integration test and examples of sdk shifted to the root of the project so that they can be recognized by the rust ecosystem.
Second commit
Second commit updates the test CI. The pulsar CI was running format, test and clippy in one job but due to addition of sdk tests which are long running, we need to have different jobs. The new jobs are basically copy of SDK's original jobs.