Skip to content

Conversation

NlightNFotis
Copy link
Member

Description

Documents the tools: input and its options for the various workflow files.

Companion PR to #2281

Fixes #1327

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@NlightNFotis NlightNFotis requested a review from a team as a code owner May 10, 2024 15:59
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.

Thanks! I made a suggestion to make it clearer the recommended case of not passing this input to let the CodeQL Action choose which tools to use.

Co-authored-by: Henry Mercer <[email protected]>
required: true
description: |
The url of codeql to use.
The version of the CodeQL tools to use. This can be any of the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an action used internally for testing only. Maybe better to change this to:

Suggested change
The version of the CodeQL tools to use. This can be any of the following:
The version of CodeQL passed to the `tools` input of the init action.

required: true
description: |
The url of codeql to use.
The version of the CodeQL tools to use. This can be any of the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above.

- A special value `linked` which uses the version of the CodeQL tools
that the Action has been bundled with.
required: false
# If not specified the Action will check in several places until it finds the CodeQL tools.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think keeping this line and removing the # is useful. At some point, but not here, we should document exactly which places are checked.

Copy link
Contributor

@henrymercer henrymercer May 13, 2024

Choose a reason for hiding this comment

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

The "check in several places" comment feels more about implementation under the hood than what the input actually means, and in fact applies even if you do specify an input. For instance, if you specify a URL we will try to extract the CodeQL version from the URL and use the toolcache if we can find a matching version. Overall, I'd prefer to remove this in favour of saying we'll use the recommended version if you don't specify an input, but I don't feel that strongly about it.

- A special value `linked` which uses the version of the CodeQL tools
that the Action has been bundled with.
required: false
# If not specified the Action will check in several places until it finds the CodeQL tools.
Copy link
Contributor

@henrymercer henrymercer May 13, 2024

Choose a reason for hiding this comment

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

The "check in several places" comment feels more about implementation under the hood than what the input actually means, and in fact applies even if you do specify an input. For instance, if you specify a URL we will try to extract the CodeQL version from the URL and use the toolcache if we can find a matching version. Overall, I'd prefer to remove this in favour of saying we'll use the recommended version if you don't specify an input, but I don't feel that strongly about it.

@NlightNFotis NlightNFotis merged commit 08487db into main May 13, 2024
@NlightNFotis NlightNFotis deleted the NlightNFotis/add_tools_help branch May 13, 2024 14:07
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.

Clarify documentation for the tools field in the init action
3 participants