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

Refactor CLI #93

Merged
merged 8 commits into from
Jun 1, 2024
Merged

Refactor CLI #93

merged 8 commits into from
Jun 1, 2024

Conversation

alilleybrinker
Copy link
Collaborator

No description provided.

alilleybrinker and others added 2 commits May 30, 2024 09:25
This commit is a pretty substantial internal refactoring of the CLI
implementation, but is purposefully designed not to break any backwards
compatibility. A number of existing flags have been deprecated and
removed from the CLI help text, but in fact are still present and work
as they did before.

This commit also introduces `hipcheck-macros`, a procedural macro helper
crate for Hipcheck.

Signed-off-by: Andrew Lilley Brinker <[email protected]>
This commit refactors the ready command to separate the different
ready-checks from the result printing. The goal here is to give a better
user experience with all results being reported together.

Signed-off-by: Andrew Lilley Brinker <[email protected]>
@alilleybrinker alilleybrinker self-assigned this May 30, 2024
@alilleybrinker alilleybrinker added the type: enhancement New feature or request label May 30, 2024
@alilleybrinker alilleybrinker added this to the 3.3.0 milestone May 30, 2024
@alilleybrinker
Copy link
Collaborator Author

This should hopefully fix the broken CLI test. It's also all been rebased on main to incorporate the ready command.

@alilleybrinker
Copy link
Collaborator Author

Haha looks like it broke some other tests. I'll have to debug it.

Signed-off-by: Andrew Lilley Brinker <[email protected]>
This commit updates the tests for the CLI to use the new `CliConfig`
API. This also involves deprecation of the old `resolve_{cache, config,
data}` functions in the `session` module, as they're no longer doing
anything especially meaningful.

Signed-off-by: Andrew Lilley Brinker <[email protected]>
Signed-off-by: Andrew Lilley Brinker <[email protected]>
Signed-off-by: Andrew Lilley Brinker <[email protected]>
@alilleybrinker
Copy link
Collaborator Author

Realized that the existing tests actually no longer made sense with the CLI refactor. I've moved the key tests into the cli module and rewritten them to properly test the new API. The key thing they're testing is just the path args, as those are the only ones with the "cascade" logic that we care about.

Signed-off-by: Andrew Lilley Brinker <[email protected]>
The `dirs` crate doesn't different config and data paths on MacOS and
Windows, so we differentiate them ourselves to make sure we're not
stuffing configuration and scripts into the same place.

Signed-off-by: Andrew Lilley Brinker <[email protected]>
@alilleybrinker alilleybrinker merged commit 2873465 into main Jun 1, 2024
9 checks passed
@alilleybrinker alilleybrinker deleted the alilleybrinker/further-cli branch June 1, 2024 19:54
@j-lanson j-lanson mentioned this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant