Skip to content

Conversation

@aeisenberg
Copy link
Contributor

@aeisenberg aeisenberg commented May 24, 2022

Also add an explanation about why codeql/javascript-experimental-atm-model isn't in the workspace file.

@aeisenberg aeisenberg requested a review from henrymercer May 24, 2022 23:52
@aeisenberg
Copy link
Contributor Author

Looks like Check synchronized files is broken on main right now. When that gets fixed, I'll re-run the job here and it should pass. The failure is unrelated to my change.

Copy link
Contributor

@henrymercer henrymercer left a comment

Choose a reason for hiding this comment

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

This was a deliberate design choice, albeit one that we could have documented better.

Most users will want to run codeql pack install to install the model pack from the registry. Users will only want to resolve the local model pack if they are running ML-powered queries with a custom model. Therefore by default we want the model pack to be resolved from the package cache.

Does .codeqlmanifest.json support JSON with comments so we can document this inline?

@aeisenberg
Copy link
Contributor Author

Does .codeqlmanifest.json support JSON with comments so we can document this inline?

No, but we probably should convert this file to codeql-workspace.yml which has the same semantics and does support comments.

The semantics are the same, except one is json, the other is
yaml.
@aeisenberg aeisenberg force-pushed the aeisenberg/manifest branch from 5897977 to 5a1663e Compare May 26, 2022 17:07
@aeisenberg
Copy link
Contributor Author

@henrymercer I changed this PR to convert to the codeql-workspace.yml file, and I added an appropriate comment.

henrymercer
henrymercer previously approved these changes May 26, 2022
Copy link
Contributor

@henrymercer henrymercer left a comment

Choose a reason for hiding this comment

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

LGTM, suggest we also update the PR description.

Co-authored-by: Henry Mercer <[email protected]>
@aeisenberg aeisenberg changed the title Update codeqlmanifest file Convert .codeqlmanifest.json file to codeql-workspace.yml May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants