Uh oh!
There was an error while loading. Please reload this page.
Fix lines of code mapping - #483
Conversation
89dcd3e to
ef9aa0aCompare| ruby: "rb", | ||
| typescript: "js", | ||
| // Language IDs used by codeql when specifying its metrics. | ||
| export type CodeQlLanguage = "cpp" | "cs" | "go" | "java" | "js" | "py"; |
There was a problem hiding this comment.
Minor: strictly speaking these are CodeQL query ID prefixes, or simply the common filename extensions for the language, and what you describe as the "Action" prefixes are really the canonical CodeQL language identifiers. Consider updating the descriptions and names accordingly.
| ruby: "rb", | ||
| typescript: "js", | ||
| // Language IDs used by codeql when specifying its metrics. | ||
| export type CodeQlLanguage = "cpp" | "cs" | "go" | "java" | "js" | "py"; |
There was a problem hiding this comment.
Also might as well keep Ruby in.
| }, | ||
| typescript: { | ||
| action: Language.javascript, | ||
| codeql: "js", |
There was a problem hiding this comment.
Ruby is not in Languages.ts in the action. So, it's not officially supported yet. Are you aware of any code scanning repos that are successfully using ruby?
There was a problem hiding this comment.
Not yet possible, but no harm being ready for it while we're modifying this code :)
887eed2 to
ec054f4CompareAdded ruby and renamed the constants. Also, confirmed that this message no longer appears in the logs. |
aeisenberg
commented
May 7, 2021
Hmmm....doesn't like ruby. https://github.com/github/codeql-action/pull/483/checks?check_run_id=2529409389#step:4:28 |
adityasharad
commented
May 7, 2021
Oh dear, bad call from me. Fine with doing Ruby separately and getting the rest of your changes underway. |
We were inadvertently using codeql language ids instead of the action's language ids. There is now a 3-way mapping between the ids used by the lines counter library, the action, and codeql.
ec054f4 to
b6ae33fCompare| dbLanguages: Language[], | ||
| logger: Logger | ||
| ): Promise<Record<string, number>> { | ||
| ): Promise<Partial<Record<IdPrefixes, number>>> { |
There was a problem hiding this comment.
I’m not sure if it makes sense to use partial here. Are you trying to describe a partial of the full matrix of supported languages?
There was a problem hiding this comment.
There are compile errors in the tests here and elsewhere.
Typescript assumes that Record<IdPrefixes, number> will have every key in IdPrefixes. Adding Partial there removes the compile errors. I was surprised by this, too. I'm not super happy about this solution. Do you know a better way of doing this?
| name: Language.javascript, | ||
| prefix: "js", | ||
| }, | ||
| }; |
There was a problem hiding this comment.
That's not a language feature we have been using in this project, but we should probably start.
Uh oh!
There was an error while loading. Please reload this page.
We were inadvertently using codeql language ids instead of the action's
language ids. There is now a 3-way mapping between the ids used by
the lines counter library, the action, and codeql.
Fixesgithub/codeql#5838
Merge / deployment checklist