Codecov Report

Merging #32 (4a5fa3b) into main (35e64f8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #32   +/-   ##
=======================================
  Coverage   81.51%   81.51%           
=======================================
  Files           8        8           
  Lines         714      714           
=======================================
  Hits          582      582           
  Misses        132      132           

The problem that sonar cloud is highlighting is not an easy fix. I need to use regex in that situation. To avoid using regex for that, I would need to replace it with a lot of new code. TBH, I think the warning is a false positive because the pattern is so long.

Thank you for making #24 happen!!

I have bypassed sonarqube false positive warining, and your chagnes looks good to me. I'd like to test it and see how it looks like in the real repo.

Just one question about naming, should we call it step-summary or job-summary? In this document https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary, the title called job summary, but it used step summary in the example code, your thought?

Yeah, I haven't gotten around to running this with the test repo yet... I should've made this PR a draft.

should we call it step-summary or job-summary?

We might not be the only step in the job that uses the GITHUB_STEP_SUMMARY env file. Calling it a "job summary" sounds like cpp-linter is the only step in the job. I chose "step summary" because the MD content that we add to GITHUB_STEP_SUMMARY is output as part of a job's summary.

It seems the summary is added for job. GITHUB_STEP_SUMMARY is collect each step summary. if in this said might make sense.

A example action you shared in #24 https://github.com/EnricoMi/publish-unit-test-result-action#github-actions-job-summary

It is a single job summary. notice the name of the job is automatically used as a label.

My real point is that other steps can also add content to GITHUB_STEP_SUMMARY that will show up in the same job summary.

example: # the job name
  steps:
    - run: echo "first step" >> $GITHUB_STEP_SUMMARY
    - run: echo "second step" >> $GITHUB_STEP_SUMMARY

This will result in the following job summary:

first step
second step

Thanks for the detailed reply, it makes more sense now.

I'm trying to add a unit test to cover the 1 new line about the "LGTM" comment...

I think the sonar cloud was trying to say "don't begin regex patterns with .*". But they used so many complex sentences that I had trouble understanding the problem. Its happy now, so I'm back to testing the intentional changes here 😥.

The PR comment looks good. The step-summary is also working as expected.

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

The test results look good 👍

I'll release minor version bump (v1.6.0) since the step summary is a new feature.

The sonar cloud linter is getting a bit ridiculous. It's latest check on main branch failed because:

1. It doesn't like regex patterns that start with . tokens
2. Now it says the logger.basicConfig() is a possible security risk.
3. The "code smells" notes are too strict.

I cannot fix point 1 without a huge rewrite.

I can probably fix point 2, but the main concern there is very context specific. I've already review the log usage, so sonar cloud doesn't need to tell me to do it again.

As for point 3, most of warnings are being relaxed via pylint config (like statement counts in a function definition) and others are just not really a problem (like renaming file variable that is namespaced by a function's scope).

TBH, I spent way more time trying to fix things in this PR that were not related to my intentional changes. 👎🏼