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

chore: update cli yargs config to highlight "ruleset*" options are mutually exclusive. #308

Merged
merged 3 commits into from
Jul 10, 2024

Conversation

zhaoyuheng200
Copy link
Member

@zhaoyuheng200 zhaoyuheng200 commented Oct 22, 2023

This should help clean up the lint function in index.js to not trying to figure out is it a path/url/encoded question.

Motivation

The current design of the lint function in index.js spends much effort trying to figure out how to ingest ruleset config correctly.
Adding conflicts in yargs creates a 3-way option exclusion, so we can have better control over CLI option intake.

I have some plan to update the lint function to call a new lintEngine function, so the existing exposed lint function will remain the same, but we'll use a new lintEngine function to separate rulesetPath, rulesetUrl and rulesetEncoded.
It would also help separate --allowPaths and future --ignorePaths option, since I don't see an easy way to add the proposed --ignorePaths easily in current setup.

Proposed Changes

Add the conflicts to create a 3-way exclusion.

Test Plan

This should be expected behavior of how customer use the repolinter cli tool, and I'm not testing yargs here.
So I didn't add any additional testing.
Passed all existing tests.

@zhaoyuheng200 zhaoyuheng200 marked this pull request as ready for review October 22, 2023 19:12
@zhaoyuheng200 zhaoyuheng200 marked this pull request as draft October 22, 2023 19:25
…ually exclusive. This should help clean up the function in index.js to *not* trying to figure out "is it a path/url/encoded" question.

Signed-off-by: Neil Zhao <[email protected]>
@hyandell
Copy link
Member

hyandell commented Jun 3, 2024

LGTM

@zhaoyuheng200 zhaoyuheng200 merged commit 30e2119 into todogroup:main Jul 10, 2024
19 checks passed
@zhaoyuheng200 zhaoyuheng200 deleted the yargs branch July 10, 2024 22:39
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