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

Implement filter logic for types and severity level #82

Merged
merged 11 commits into from Mar 4, 2019

Conversation

Projects
None yet
4 participants
@Raynos
Copy link
Contributor

Raynos commented Feb 28, 2019

  • add --filter=compliance
  • add --filter=security
  • add --filter=high / --filter=h

@Raynos Raynos referenced this pull request Feb 28, 2019

Merged

Add the ability to filter report #79

3 of 3 tasks complete
@Raynos

This comment has been minimized.

Copy link
Contributor Author

Raynos commented Feb 28, 2019

image

This is what the filtering does.

@Fishrock123

This comment has been minimized.

Copy link
Contributor

Fishrock123 commented Feb 28, 2019

Tests fail but it looks like it'l work. Will take a look tomorrow morning before I OOO.

@Raynos Raynos force-pushed the filter-compliance branch from 4042209 to 969cdfa Mar 1, 2019

@Raynos Raynos force-pushed the filter-level branch from e53d5bf to 2ec9e7b Mar 1, 2019

@Raynos Raynos marked this pull request as ready for review Mar 1, 2019

@Raynos

This comment has been minimized.

Copy link
Contributor Author

Raynos commented Mar 1, 2019

What should the semantics of $ ncm-cli report ./test/fixtures/mock-project/ --security --compliance be ?

The current implementation is AND instead of OR.

@Fishrock123
Copy link
Contributor

Fishrock123 left a comment

Seems fine once the other lands. You may also want to squash here and just keep the commits and test updates roughly in sync if they aren't.

@@ -125,6 +125,8 @@ function optionsList () {
{${COLORS.teal} -l, --long} {white Expanded output with module list}
{${COLORS.teal} -c --compliance} {white Expanded output with compliance failures}
{${COLORS.teal} -s --security} {white Expanded output with security failures}
{${COLORS.teal} --filter=<value> {white Expanded output with filtered module list.}}
{white filters can be critical, high, medium or low}

This comment has been minimized.

@juliangruber

juliangruber Mar 3, 2019

Member
Suggested change
{white filters can be critical, high, medium or low}
{white filters can be 'critical', 'high', 'medium' or 'low'}

Should we quote the possible values?

}
let minimumSeverity = options.filterLevel
for (const pkg of report) {
let hasComplianceFailure = pkg.scores.some(

This comment has been minimized.

@juliangruber

juliangruber Mar 3, 2019

Member
Suggested change
let hasComplianceFailure = pkg.scores.some(
const hasComplianceFailure = pkg.scores.some(

This comment has been minimized.

@Raynos

Raynos Mar 3, 2019

Author Contributor

we should enable prefer-const rule in eslint ( https://eslint.org/docs/rules/prefer-const ).

@Raynos Raynos force-pushed the filter-compliance branch from 969cdfa to a226fb8 Mar 4, 2019

@Raynos Raynos force-pushed the filter-level branch from 16302b9 to f8f7d34 Mar 4, 2019

@Raynos Raynos changed the base branch from filter-compliance to master Mar 4, 2019

@Raynos Raynos force-pushed the filter-level branch from 46bb370 to bc23401 Mar 4, 2019

@Raynos Raynos merged commit 3d858b7 into master Mar 4, 2019

1 check passed

CodeBuild Build was a success.
Details

@Raynos Raynos deleted the filter-level branch Mar 4, 2019

@Jazneil Jazneil added this to the Sprint 10 (2/26 - 3/18) milestone Mar 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.