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

Add --extra-args to fix #298 #300

Merged
merged 4 commits into from
Jan 9, 2023
Merged

Add --extra-args to fix #298 #300

merged 4 commits into from
Jan 9, 2023

Conversation

sargunv
Copy link
Contributor

@sargunv sargunv commented Jan 9, 2023

As discussed in #298, this PR adds a --extra-args flag to pass additional arguments to the underlying audit command.

Since these additional args are likely to include flags (--foo), I added an escaping mechanism, so any "extra arg" starting with a \ will have that first \ removed.

Usage example:

npx audit-ci@^6 --extra-args '\--exclude' '@sargunv/testlib-c'

Signed-off-by: Sargun Vohra <sargun.vohra@convoy.com>
Signed-off-by: Sargun Vohra <sargun.vohra@convoy.com>
@quinnturner
Copy link
Member

I also had trouble updating Yarn Berry to 3.3.1 in #294. Unless necessary, let's keep 3.3.0 and push the investigation for why 3.3.1 isn't working.

@quinnturner
Copy link
Member

Can you add a test to cover the --exclude flag using the new --extra-args flag? We currently don't have integration tests for CLI usage, so using the normal unit tests with the config helpers is sufficient coverage. I will verify it works using the CLI locally for this PR once the test it written :)

Signed-off-by: Sargun Vohra <sargun.vohra@convoy.com>
Signed-off-by: Sargun Vohra <sargun.vohra@convoy.com>
@@ -385,6 +386,23 @@ Or, with the CLI:
npx audit-ci@^6 --report-type summary
```

### Pass additional args to Yarn to exclude a certain package from audit
Copy link
Member

Choose a reason for hiding this comment

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

Love it!

@sargunv
Copy link
Contributor Author

sargunv commented Jan 9, 2023

Can you add a test to cover the --exclude flag using the new --extra-args flag? We currently don't have integration tests for CLI usage, so using the normal unit tests with the config helpers is sufficient coverage. I will verify it works using the CLI locally for this PR once the test it written :)

Currently the test Yarn Berry version is v2.4, so --exclude isn't available, but I wrote a test with --environment instead (basically the same test as skip-dev but using extra-args instead).

I was thinking I'd file a separate PR to include Yarn v3 in addition to v2 in tests (maybe v4 rc builds too?) and run against them all. Unsure if that's worth it though, thoughts?

@quinnturner
Copy link
Member

Currently the test Yarn Berry version is v2.4, so --exclude isn't available, but I wrote a test with --environment instead (basically the same test as skip-dev but using extra-args instead).

👍🏻

I was thinking I'd file a separate PR to include Yarn v3 in addition to v2 in tests (maybe v4 rc builds too?) and run against them all. Unsure if that's worth it though, thoughts?

It was on my radar to do this as well. IMO, it is worth it, since the point of this package is to cover all package managers. However, I understand if you have a lot on your plate right now to tackle it. In either case, consider filing an issue so that we can track that work 😄

@sargunv
Copy link
Contributor Author

sargunv commented Jan 9, 2023

It was on my radar to do this as well. IMO, it is worth it, since the point of this package is to cover all package managers. However, I understand if you have a lot on your plate right now to tackle it. In either case, consider filing an issue so that we can track that work 😄

Cool, filed #302. I might work on it at some point, but not in the very near future.

@sargunv sargunv marked this pull request as ready for review January 9, 2023 20:31
@quinnturner quinnturner self-requested a review January 9, 2023 20:31
Copy link
Member

@quinnturner quinnturner left a comment

Choose a reason for hiding this comment

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

Everything looks great. Thank you for this! I will release it ASAP. Will tag back here once released.

Closes #298

@quinnturner quinnturner merged commit 709bf95 into IBM:main Jan 9, 2023
quinnturner added a commit to quinnturner/audit-ci that referenced this pull request Jan 9, 2023
chore(deps): bump json5 from 1.0.1 to 1.0.2 (IBM#299)
Add --extra-args to fix IBM#298 (IBM#300)
@quinnturner quinnturner mentioned this pull request Jan 9, 2023
quinnturner added a commit to quinnturner/audit-ci that referenced this pull request Jan 9, 2023
chore(deps): bump json5 from 1.0.1 to 1.0.2 (IBM#299)
Add --extra-args to fix IBM#298 (IBM#300)

Signed-off-by: Quinn Turner <quinnturnertech@gmail.com>
@quinnturner
Copy link
Member

Released in v6.6.0

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