Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

CLI Reform with cargo decouple #226

Closed
pinkforest opened this issue Dec 16, 2021 · 1 comment
Closed

CLI Reform with cargo decouple #226

pinkforest opened this issue Dec 16, 2021 · 1 comment
Assignees
Labels
rfc Request for comments
Milestone

Comments

@pinkforest
Copy link
Collaborator

pinkforest commented Dec 16, 2021

This comment was made as part of 0.58.0 bump via 0.11.2 release

See 0.12.0 major changes from the next comment

See #244 for general discussion re: changes outside sub-issues - I've limited this issue to keep track of agreed changes

As part of working Cargo bump at #208

I came across the bit that builds the CompileOptions I have to adjust for cargo >= 0.58.0

0.52.0 CompileOptions - https://docs.rs/cargo/0.52.0/cargo/ops/struct.CompileOptions.html
0.58.0 CompileOptions - https://docs.rs/cargo/latest/cargo/ops/struct.CompileOptions.html

For the next minor release I would suggest we don't adjust any CLI options which are currently

        --features <FEATURES>     Space-separated list of features to activate.
        --all-features            Activate all available features.
        --no-default-features     Do not activate the `default` feature.

Questions

  • I am not sure whether anyone runs cargo geiger on CI or something i.e. if we introduce breaking change to CLI options in future? I heavily doubt there were any other than new options but for as the future expectation i am most concerned with.
  • Do people expect these CLI options to stay same or follow cargo ones 1:1 both the way they are named and set ?
  • Would it be preferable to match cargo build options 1:1 and create as expectation how we follow cargo release?

Approach Proposed

  • For now I will just translate the current arguments to current structures and use the new constructor functions provided
  • I will use CliFeatures::from_command_line to construct the features
  • For the next major version de-couple / reduce translation between CLI and cargo so it's easier to change that dependency.

Notes

  • Cargo has comma/space separated list whilst cargo geiger has space separated
    --features <FEATURES>... Space or comma separated list of features to activate
  • We will have to handle errors better in future e.g. from from_command_line but it will require some overall reforms.
@pinkforest pinkforest added the rfc Request for comments label Jan 5, 2022
@pinkforest pinkforest self-assigned this Jan 5, 2022
This was referenced Jan 6, 2022
@pinkforest pinkforest added this to the M-0.12.0 milestone Jan 6, 2022
@pinkforest
Copy link
Collaborator Author

pinkforest commented Jan 7, 2022

Okay for next major we need a logical way to distinguish between geiger and cargo options

Targeting major 0.12.0 release with all these breaking changes

Please open an issue for any proposed / desired change and then refer back to this tracker :)

I am proposing the below

cargo geiger build -- should do build deps

cargo geiger test -- should do test deps

cargo geiger run -- should do runtime deps

cargo geiger scan -- Scan only

This would mean that we remove --build-dependencies and --dev-dependencies

And introducing different run types which have own separate report output mirroring cargo usage

Essentially

cargo geiger

Question becomes where do we intro geiger options and best way to go by with it?

Let's look at what we need to support for geiger opts

We need the below geiger specific options

#190 - -dX | --depth X (Proposed) Depth of dependencies (New in 0.12.0)

@pinkforest pinkforest changed the title CompileOptions Reform with cargo bump CLI Reform with cargo decouple Jan 7, 2022
@pinkforest pinkforest pinned this issue Jan 7, 2022
@geiger-rs geiger-rs locked and limited conversation to collaborators Jan 7, 2022
@pinkforest pinkforest converted this issue into discussion #312 May 17, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
rfc Request for comments
Projects
None yet
Development

No branches or pull requests

1 participant