Add threads flag to analyze action #73
Conversation
alexkappa
force-pushed
the
add-analyze-threads-flag
branch
from
6fc27f9 to
2758bd3
Compare
| fs.rmdirSync(tmpDir, { recursive: true }); | ||
| return result; | ||
| } | ||
|
|
There was a problem hiding this comment.
It seems a bit odd to move these to util as they are specific to the CodeQL analysis step.
There was a problem hiding this comment.
Indeed, my first instinct was to create a finalize-db.test.ts file and add all relevant test cases in there. That gave me some trouble unfortunately which CodeQL was the only thing that pointed to the problem: the run() function at the end of finalize-db.ts.
Now as to why config.ts in particular, I just saw other similar functions and thought it might be alright.
Ideally I would have liked to have this as follows:
<action-name>/action.yml -> lib/<action-name>.js
lib/<action-name>.js -> lib/<library-or-package>.js
But it would be quite a bigger change and one that merits discussion first. What are your thoughts about this?
|
|
||
| for (const [input, expectedFlag] of Object.entries(tests)) { | ||
|
|
||
| process.env['INPUT_RAM'] = input; |
There was a problem hiding this comment.
Glad you worked out how to do this. I also added a function to do this at https://github.com/github/codeql-action/blob/main/src/config-utils.test.ts#L8
Would be good to consolidate these into one place. Perhaps we need a new file for utilities for use during tests. Then this function, and withTmpDir could move there.
There was a problem hiding this comment.
Ah, seems @sampart actually introduced a file like this in https://github.com/github/codeql-action/pull/75/files#diff-2bc785dec81a33ef3bbc6dbe25f2f8c5
For this PR perhaps just ignore this comment then and we'll come back to address stuff in a later PR once all the current ones are merged.
There was a problem hiding this comment.
Ah great, I'll be sure to use that to be more consistent. It would surely give more context.
Description
Adds an
inputs.threadsflag to the analyze command. Usage is as follows:The value of
threadsdefaults to1if is not specified. If specified, will be capped to the vale ofos.cpus().length.Merge / deployment checklist