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

Remove LoC metrics from the analysis summary #14811

Merged

Conversation

henrymercer
Copy link
Contributor

Currently for C/C++, JavaScript/TypeScript, and Python, we print a table like the following after analyzing the database:

Analysis produced the following metric data:

|                  Metric                   | Value  |
+-------------------------------------------+--------+
| Total lines of C/C++ code in the database | 490947 |

We want to stop printing this lines of code information now that we have replaced it by file coverage information. We have already removed queries tagged lines-of-code from the analysis summary, but C/C++, JavaScript/TypeScript, and Python define both lines of code and lines of user code queries, and only the lines of user code queries are tagged lines-of-code. Therefore, this PR tags the lines of code queries with telemetry to remove them from the analysis summary, while preserving the metric data in the SARIF file for advanced users.

@henrymercer
Copy link
Contributor Author

Reviewers, do you think this change should have a changenote?

@erik-krogh
Copy link
Contributor

Reviewers, do you think this change should have a changenote?

No. There is no change to query results, or to our QL libraries.

@erik-krogh erik-krogh added the no-change-note-required This PR does not need a change note label Nov 16, 2023
@MathiasVP
Copy link
Contributor

I'd say so, yes. Not sure if it should be a CLI change note, though?

@RasmusWL
Copy link
Member

now that we have replaced it by file coverage information

Are you referring to this bit?

CodeQL scanned 17 out of 17 Python files and 0 out of 1 JavaScript files in this job. Typically CodeQL is configured to analyze a single CodeQL language per job, so check the status page for overall coverage information across all jobs: https://github.com/<repo>/security/code-scanning/tools/CodeQL/status/

Copy link
Member

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for Python

@RasmusWL
Copy link
Member

I'd say so, yes. Not sure if it should be a CLI change note, though?

Agreed that if you want to add a change-note, it should go to the CLI (or action)

@henrymercer henrymercer merged commit 0c1fb8c into main Nov 16, 2023
17 of 18 checks passed
@henrymercer henrymercer deleted the henrymercer/remove-lines-of-non-user-code-from-summary branch November 16, 2023 12:30
@mat-sylvia-mark43
Copy link

@henrymercer I'm not able to find this information anywhere for javascript-typescript. I see it in Java CodeQL jobs. When I view the coverage page as described above it just shows file counts not LOC? Where can I find this data for javascript-typescript?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ JS no-change-note-required This PR does not need a change note Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants