dart-code-checker / dart-code-metrics

Software analytics tool that helps developers analyse and improve software quality.
https://dcm.dev
Other
860 stars 265 forks source link

[BUG] Gitlab reporter prints logs and JSON instead of JSON alone #1151

Open pwa-tapptic opened 1 year ago

pwa-tapptic commented 1 year ago

What did you do? Please include the source code example causing the issue. I run flutter pub run dart_code_metrics:metrics lib --reporter=gitlab > gitlab.json based on the documentation here.

This issue is version-specific, looks like it does not depend on any specific configuration.

What did you expect to happen? As it is demonstrated in the documentation I am supposed to get valid JSON report (gitlab.json) with Gitlab style when I use gitlab reporter.

What actually happened? Versions tested:

[{}, {}] <- JSON ISSUES HERE

🆕 Update available! 4.21.3 -> 5.4.0 🆕 Changelog: https://github.com/dart-code-checker/dart-code-metrics/releases/tag/5.4.0



**Are you willing to submit a pull request to fix this bug?**
Unfortunately no.
dkrutskikh commented 1 year ago

@pwa-tapptic can you provide gitlab.json ?

pwa-tapptic commented 1 year ago

@dkrutskikh it's actually printed as "Example bad JSON file" in my first message in What actually happened? section.

incendial commented 1 year ago

@pwa-tapptic there are 2 options here: either hide progress indication if the reporter is not a console reporter or add an option of the output file name. Is the progress indication valuable to you to see or it can be omitted?

pwa-tapptic commented 1 year ago

@incendial IMO there is also a third option you may consider, namely using STDOUT to print JSON and STDERR to print progress (I believe that git and many other tools do this as well - they distinguish STDOUT and STDERR and print some messages, not necessarily errors onto STDERR).

Given simple script

echo "message to stdout"
1>&2 echo "msg to STDERR"

when the script is ran then only msg to STDERR is printed and result.txt contains the STDOUT

$ ./script.sh > result.txt
msg to STDERR
$ cat result.txt 
message to stdout
incendial commented 1 year ago

Hmm, interesting suggestion, actually, thank you!

incendial commented 1 year ago

Now I'm curious how many systems treat anything printed to stderr as an exit. So, this might introduce a breaking change

incendial commented 1 year ago

@pwa-tapptic so a workaround would be to pass --no-congratulate as it should suppress all the messages, while we're fixing the issue

pwa-tapptic commented 1 year ago

I can confirm that adding --no-congratulate at the end of the command works as a workaround 👍

As for the stderr and exiting - it's something to be checked but AFAIK those are two separate things and play separate roles in "CLI app contract", so to speak. You may print plenty of stderr messages (even pure errors), but still exit with 0. You may also print nothing but exit with 1. There are some specific cases where you may have scripts that "listen to" stderr and perform actions based on this, but this is something that needs to be intentionally configured or implemented. Not sure if it could be somehow useful in Flutter and DCM case. I can recall that i.e. in Azure DevOps you may add a script step and enabled an option that will fail the step if something has been printed to stderr. It's an advanced option disabled by default, I can imagine cases when it could be helpful, but I can't imagine how it can be useful in context of Flutter and DCM, especially because it's not that uncommon to use both streams to print messages.