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

ci: test misc/keygen #4370

Open
mxinden opened this issue Aug 21, 2023 · 5 comments
Open

ci: test misc/keygen #4370

mxinden opened this issue Aug 21, 2023 · 5 comments

Comments

@mxinden
Copy link
Member

mxinden commented Aug 21, 2023

Description

Given that keygen is set to publish = false it is excluded from most CI steps.

publish = false

gather_published_crates:
runs-on: ubuntu-latest
outputs:
members: ${{ steps.cargo-metadata.outputs.members }}
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@stable
- id: cargo-metadata
run: |
WORKSPACE_MEMBERS=$(cargo metadata --format-version=1 --no-deps | jq -c '.packages | map(select(.publish == null) | .name)')
echo "members=${WORKSPACE_MEMBERS}" >> $GITHUB_OUTPUT

Motivation

keygen is a useful tool worth testing in our CI.

Are you planning to do it yourself in a pull request?

No

@mxinden
Copy link
Member Author

mxinden commented Aug 21, 2023

@thomaseizinger you mentioned you had an idea on how to best achieve this in #4311 (comment). Mind posting it here?

@mxinden mxinden added difficulty:easy help wanted getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Aug 21, 2023
@thomaseizinger
Copy link
Contributor

My idea was to restructure the repository into the following directories:

  • apps: Contains all binaries, like libp2p-server but also examples. Examples would end in -example.
  • plugins: Contains all implementations of NetworkBehaviour
  • transports: Contains all transports, unchanged from what we currently have.
  • tests: Contains things like interop-tests, wasm-tests, test-muxer-harness, swarm-test
  • muxers: Unchanged.

We can retain misc for now for all the others that don't fit.

@mxinden
Copy link
Member Author

mxinden commented Sep 1, 2023

apps: Contains all binaries, like libp2p-server but also examples. Examples would end in -example.

I think there is great value in the Rust repository convention of an examples/ folder. Looking at the GitHub traffic statistics, this is a highly visited path. With that in mind, I don't think we should have our examples in any other folder than the examples/ folder.

plugins: Contains all implementations of NetworkBehaviour

We currently have a mismatch where NetworkBehaviours are stored in a folder called protocols/. I don't think we should change that mismatch with a new one where all NetworkBehaviours are stored in a folder called plugins/. In other words I think the rename from NetworkBehaviour to Plugin should come first.

tests: Contains things like interop-tests, wasm-tests, test-muxer-harness, swarm-test

Sounds good to me.


I don't see enough signals from others outside of the core maintainers to give this a high priority though.

@thomaseizinger
Copy link
Contributor

plugins: Contains all implementations of NetworkBehaviour

We currently have a mismatch where NetworkBehaviours are stored in a folder called protocols/. I don't think we should change that mismatch with a new one where all NetworkBehaviours are stored in a folder called plugins/. In other words I think the rename from NetworkBehaviour to Plugin should come first.

Is the mismatch of plugins any different from protocols? At least with plugins, we could move libp2p-connection-limits etc into that directory and also libp2p-request-response would make more sense in there.

I don't see enough signals from others outside of the core maintainers to give this a high priority though.

We are the ones interacting the most with this directory structure, i.e. we are its heaviest users.

apps: Contains all binaries, like libp2p-server but also examples. Examples would end in -example.

I think there is great value in the Rust repository convention of an examples/ folder.

We aren't really following that convention though, in other words, cargo can't deal with it as far as I know? We could also have a separate directory bins that contains non-example projects.

@thomaseizinger thomaseizinger removed getting-started Issues that can be tackled if you don't know the internals of libp2p very well difficulty:easy help wanted labels Sep 12, 2023
@thomaseizinger
Copy link
Contributor

Untagging this because it is not yet actionable.

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

No branches or pull requests

2 participants