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

Make CLI write reports to file for scanner jobs #250

Merged
1 commit merged into from
Apr 18, 2023
Merged

Conversation

chrisgacsal
Copy link
Contributor

@chrisgacsal chrisgacsal commented Apr 14, 2023

Description

Currently the CLI writes reports to the console by default which might make sense when the CLI is used locally. However it makes hard to spot issues/errors at runtime for scanner jobs as it pollutes the logs with the reports of the scanners.
This change makes the the scanner job to write the scanner reports to files at path provided via --output command line parameter.

Type of Change

[ ] Bug Fix
[ ] New Feature
[ ] Breaking Change
[x] Refactor
[ ] Documentation
[ ] Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

@chrisgacsal chrisgacsal requested a review from a team as a code owner April 14, 2023 08:59
@ghost
Copy link

ghost commented Apr 14, 2023

Change is good, but I think the description needs a tweak, this doesn't change the default in the CLI, it changes the scanner job.

@chrisgacsal chrisgacsal changed the title Make CLI write reports to file by default Make CLI write reports to file for scanner jobs Apr 14, 2023
@chrisgacsal
Copy link
Contributor Author

Change is good, but I think the description needs a tweak, this doesn't change the default in the CLI, it changes the scanner job.

@sambetts-cisco Updated the description for the PR.

@chrisgacsal chrisgacsal force-pushed the cli-report-to-file branch 2 times, most recently from 6c71bf6 to 3442986 Compare April 14, 2023 15:52
@chrisgacsal chrisgacsal requested a review from a user April 14, 2023 17:22
@chrisgacsal chrisgacsal force-pushed the cli-report-to-file branch 2 times, most recently from fbf566a to 4cf4729 Compare April 17, 2023 12:32
Copy link
Member

@pbalogh-sa pbalogh-sa 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 @chrisgacsal

@ghost ghost merged commit a7270ef into main Apr 18, 2023
5 checks passed
@ghost ghost deleted the cli-report-to-file branch April 18, 2023 08:05
This pull request was closed.
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