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

[#98] add CLI tools to crates.io publish script #379

Conversation

orecham
Copy link
Contributor

@orecham orecham commented Sep 12, 2024

Notes for Reviewer

  1. put all CLI tools in a single easy-to-install crate iceoryx2-cli
  2. add a README.md for the CLI crate
  3. add a markdownlint configuration to the project
    1. note that not all markdown files have been formatted, left for a separate PR
    2. at the very least it will ensure future markdown files are consistent
    3. rules can be adapted as we go
  4. rename CLI tools
    1. iox2-services -> iox2-service
    2. iox2-nodes -> iox2-node
  5. add the crate with CLI tools to script for publishing to crates.io
  6. remove cargo crates of unimplemented CLI tools
    1. iox2-introspect
    2. iox2-processes
    3. iox2-pub
    4. iox2-sub
    5. iox2-rpc
  7. update release notes with the tools that are implemented

Pre-Review Checklist for the PR Author

  1. Add sensible notes for the reviewer
  2. PR title is short, expressive and meaningful
  3. Relevant issues are linked in the References section
  4. Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
  5. Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  6. Commits messages are according to this guideline
  7. Tests follow the best practice for testing
  8. Changelog updated in the unreleased section including API breaking changes
  9. Assign PR to reviewer
  10. All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

Relates #98

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.47%. Comparing base (0d14d50) to head (fdb27fe).
Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #379      +/-   ##
==========================================
- Coverage   79.48%   79.47%   -0.01%     
==========================================
  Files         193      193              
  Lines       22716    22716              
==========================================
- Hits        18055    18054       -1     
- Misses       4661     4662       +1     

see 6 files with indirect coverage changes

@@ -32,3 +32,5 @@ cargo publish -p iceoryx2-bb-memory
cargo publish -p iceoryx2-cal
cargo publish -p iceoryx2-bb-trait-tests
cargo publish -p iceoryx2
cargo publish -p iox2
Copy link
Contributor

Choose a reason for hiding this comment

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

If we publish this and the services, we also need a little readme describing how to install them. I think cargo install iox2 and cargo install iox2-services should work. This could be placed into iceoryx2-cli/README.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elfenpiff It's not clear exactly what you expect. Do you have an example?

Copy link
Contributor

Choose a reason for hiding this comment

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

I try to phrase it as a use case so that it is better understandable.

  • As a iceoryx2 user,
  • that uses iceoryx2 as a dependency in my projects workspace Cargo.toml
  • I want install the iox2 cli
  • So that I can list all services and nodes.

So the question is, how does the iceoryx2 user install iox2 cli with all of its subcommand.

  1. cargo run --bin iox2 -- services list does not work -> Failed to execute command: Command not found: services
  2. cargo run --bin iox2-services -- list -> works but is not the intended use case.

So I would like to have a README.md document inside iceoryx2-cli/iox2, since this is the document that is shown in crates.io when the crate is published, see: https://crates.io/crates/iceoryx2

In this document there is a installation guide with terminal commands on how to install the iox2 cli with all of its subcommands so that the subcommands are installed in the right path and the user can just use it as intended.

I assume that the intended use case is, that iox2 is in the PATH of the shell and the user can just call

iox2 services list

for instance.

Copy link
Contributor

Choose a reason for hiding this comment

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

@elBoberido whats your take on this?

Copy link
Member

Choose a reason for hiding this comment

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

I guess you mean something like https://crates.io/crates/cargo-outdated or https://crates.io/crates/cargo-list. Without a README.md in the folder, the page on crates.io will look like this https://crates.io/crates/iceoryx2-bb-container

I think havin a small README.md for the cli tools would be helpful for people looking for them on crates.io. It does not need to be fancy in the beginning, just a hint on how to install and use them, maybe with an output of a cli tool as example.

Copy link
Contributor Author

@orecham orecham Sep 20, 2024

Choose a reason for hiding this comment

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

@elfenpiff @elBoberido I think I understand my confusion now, there are actually two things being asked:

  1. How does the user install the iox2 CLI tools
  2. How does the user use the CLI tools

For (1) I can add a short readme to the crates, but it will essentially just boil down to cargo install iox2. I did some research and might need to tweak the Cargo.tml file slightly to make this work. I can also make it that these are automatically installed when doing cargo install iceoryx2 as well.

For (2), the user should use iox2 --help. Writing documentation (like in https://crates.io/crates/cargo-list) will be tedious to maintain IMO. I can add the instruction explaining to the user to use iox2 --help (or equivalent) to the readmes as a compromise.

Copy link
Contributor Author

@orecham orecham Sep 20, 2024

Choose a reason for hiding this comment

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

So the question is, how does the iceoryx2 user install iox2 cli with all of its subcommand.

cargo run --bin iox2 -- services list does not work -> Failed to execute command: Command not found: services
cargo run --bin iox2-services -- list -> works but is not the intended use case.

I replied with the reasoning for this in a comment in another PR, but if you do cargo run --bin iox2 -- --help, the reason it did not work will be clear and the way to run the sub-commands will be shown.

Copy link
Contributor

@elfenpiff elfenpiff Sep 20, 2024

Choose a reason for hiding this comment

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

@orecham I just want to have a readme for 1. How does the user install the iox2 CLI tools. For 2. How does the user use the CLI tools the user can call iox2 --help.

In the end the iox2 --help command shall deliver some examples how to use it. For instance rm --help lists at the end some example how to remove files with certain properties.

Copy link
Member

Choose a reason for hiding this comment

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

I think a short description at the top what the tool does would be helpful, together with a link to the iceoryx2 project. Followed by how to install and how to use. For the how to use section, the most common usage should be shown, e.g. iox2 services and how to discover more options, e.g. iox servives --help. The output of the most common command would also be nice but not a must have.

Copy link
Contributor Author

@orecham orecham Sep 21, 2024

Choose a reason for hiding this comment

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

@elfenpiff @elBoberido I moved all tools into a single crate for easy installation and added a short readme for that crate

you can test installation locally:

$ cargo install --path ./iceoryx2-cli

users should be able to do:

$ cargo install iceoryx2-cli

there were some other tweaks, see the pr description for details

@orecham orecham force-pushed the iox2-98-add-iox2-cli-binaries-to-cargo-publish-script branch from f33c252 to 56b296e Compare September 15, 2024 22:58
@orecham orecham changed the title [#98] Add iox2 and iox2-services to cargo publish script [BASED ON #380] [#98] Add iox2 and iox2-services to cargo publish script Sep 15, 2024
@orecham orecham force-pushed the iox2-98-add-iox2-cli-binaries-to-cargo-publish-script branch from 56b296e to 506c032 Compare September 15, 2024 23:03
@orecham orecham changed the title [BASED ON #380] [#98] Add iox2 and iox2-services to cargo publish script [BASED ON #380] [#98] Add CLI tools to crates.io publish script Sep 15, 2024
@orecham orecham changed the title [BASED ON #380] [#98] Add CLI tools to crates.io publish script [BASED ON #380] [#98] add CLI tools to crates.io publish script Sep 15, 2024
@orecham orecham force-pushed the iox2-98-add-iox2-cli-binaries-to-cargo-publish-script branch from 1512f6f to c99c1bc Compare September 20, 2024 11:14
@orecham orecham changed the title [BASED ON #380] [#98] add CLI tools to crates.io publish script [#98] add CLI tools to crates.io publish script Sep 20, 2024
@orecham orecham force-pushed the iox2-98-add-iox2-cli-binaries-to-cargo-publish-script branch from 77e13ad to 279a355 Compare September 21, 2024 01:44
@orecham orecham force-pushed the iox2-98-add-iox2-cli-binaries-to-cargo-publish-script branch 8 times, most recently from bb9420f to 5e4693e Compare September 21, 2024 02:53
@orecham orecham force-pushed the iox2-98-add-iox2-cli-binaries-to-cargo-publish-script branch from 5e4693e to fdb27fe Compare September 21, 2024 03:07
.markdownlint.yaml Show resolved Hide resolved
@orecham orecham merged commit 59b94a0 into eclipse-iceoryx:main Sep 21, 2024
54 of 55 checks passed
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