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

Enabling SARIF_REPORTER overrides TEXT_REPORTER features #1822

Closed
bdovaz opened this issue Sep 1, 2022 · 4 comments · Fixed by #1825
Closed

Enabling SARIF_REPORTER overrides TEXT_REPORTER features #1822

bdovaz opened this issue Sep 1, 2022 · 4 comments · Fixed by #1825
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@bdovaz
Copy link
Collaborator

bdovaz commented Sep 1, 2022

If I activate SARIF_REPORTER and TEXT_REPORTER at the same time, the following happens:

  1. A folder "linters_logs" is created with all linters logs in *.log format (CORRECT).
  2. A folder "sarif" is created with all the linters logs in *.sarif format (CORRECT).
  3. A file "megalinter-report.sarif" is created in the root replacing the file "megalinter-report.log" (WRONG).

Why do I lose the complete log in text format which is equivalent to what is shown by console? It is very useful to inspect it later.

@bdovaz bdovaz added the bug Something isn't working label Sep 1, 2022
@nvuillam
Copy link
Member

nvuillam commented Sep 1, 2022

When we use SARIF reporter, we call the linters with arguments telling them to output the result in SARIF and not in "human readable" text

Calling twice the linters with text mode then SARIF mode would double the performances and that would be not acceptable, but I recently found a library that seems to be able to convert SARIF to human text, i've not tested it yet but that could be an interesting evolution to implement someday on MegaLinter :)

@bdovaz
Copy link
Collaborator Author

bdovaz commented Sep 1, 2022

Well, in the same way that you documented several topics about the TAP reporter, it should be documented that there are reporters that can "coexist" at the same time but others, as in this case TEXT vs SARIF, cannot. That is, if SARIF is activated, the text reporter stops working.

@Kurt-von-Laven
Copy link
Collaborator

Happy to accept a pull request to clarify this in the documentation. On a related note, I see that docs/reporters.md is currently missing SARIF, so that is another place to update in addition to docs/reporters/SarifReporter.md.

@Kurt-von-Laven Kurt-von-Laven added documentation Improvements or additions to documentation good first issue Good for newcomers and removed bug Something isn't working labels Sep 1, 2022
@nvuillam
Copy link
Member

nvuillam commented Sep 1, 2022

Well, in the same way that you documented several topics about the TAP reporter, it should be documented that there are reporters that can "coexist" at the same time but others, as in this case TEXT vs SARIF, cannot. That is, if SARIF is activated, the text reporter stops working.

Text reporter just store the console output of linters, so technically it still works even if the content is sarif :D
But I get your point, I hope that someday i'll find the time to implement the sarif-to-human for such case ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants