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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

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 <neil.leopard@gmail.com>
@hyandell
Copy link
Member

hyandell commented Jun 3, 2024

LGTM

zhaoyuheng200 and others added 2 commits June 4, 2024 21:57
* removed Ruby setup. I'm not sure what's usage of ruby here other than execute `npm test`.
verified the node test cases works correctly in ubuntu/mac
* remove node 14 support.
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.

None yet

2 participants