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

Migrate compute_ctl arg parsing to clap derive #10497

Merged
merged 1 commit into from
Jan 31, 2025
Merged

Conversation

tristan957
Copy link
Member

@tristan957 tristan957 commented Jan 23, 2025

The primary benefit is that all the ad hoc get_matches() calls are no longer necessary. Now all it takes to get at the CLI arguments is referencing a struct member. It's also great the we can replace the ad hoc CLI struct we had with this more formal solution.

@tristan957 tristan957 requested a review from a team as a code owner January 23, 2025 19:41
@tristan957 tristan957 requested review from hlinnaka and MMeent January 23, 2025 19:41
Copy link

github-actions bot commented Jan 23, 2025

7414 tests run: 7062 passed, 0 failed, 352 skipped (full report)


Code coverage* (full report)

  • functions: 33.3% (8515 of 25552 functions)
  • lines: 49.1% (71424 of 145420 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
6aa1d33 at 2025-01-31T19:01:53.700Z :recycle:

@tristan957 tristan957 force-pushed the tristan957/cli branch 2 times, most recently from c322202 to 4748e2e Compare January 27, 2025 17:35
@tristan957 tristan957 force-pushed the tristan957/cli branch 2 times, most recently from 87fb00f to 6c49da1 Compare January 30, 2025 00:37
Copy link
Contributor

@myrrc myrrc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the cleanup

@tristan957
Copy link
Member Author

Thank you for the review ❤️

@tristan957 tristan957 added this pull request to the merge queue Jan 31, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 31, 2025
@tristan957 tristan957 added this pull request to the merge queue Jan 31, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 31, 2025
The primary benefit is the all the ad hoc get_matches() calls are no
longer necessary. Now all it takes to get at the CLI arguments is
referencing a struct member.

Signed-off-by: Tristan Partin <[email protected]>
@tristan957
Copy link
Member Author

Rebasing on main to see if it fixes the merge queue issue

@tristan957 tristan957 added this pull request to the merge queue Jan 31, 2025
Merged via the queue into main with commit fcd195c Jan 31, 2025
86 checks passed
@tristan957 tristan957 deleted the tristan957/cli branch January 31, 2025 19:11
winter-loo pushed a commit to winter-loo/neon that referenced this pull request Feb 4, 2025
The primary benefit is that all the ad hoc get_matches() calls are no
longer necessary. Now all it takes to get at the CLI arguments is
referencing a struct member. It's also great the we can replace the ad
hoc CLI struct we had with this more formal solution.

Signed-off-by: Tristan Partin <[email protected]>
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.

2 participants