docs icon indicating copy to clipboard operation
docs copied to clipboard

CodeQL status check name is wrong

Open mario-campos opened this issue 3 years ago • 13 comments

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/triaging-code-scanning-alerts-in-pull-requests#code-scanning-results-check

What part(s) of the article would you like to see updated?

Any references to "Code Scanning results" should be changed to reflect the new status check name "CodeQL".

Additional information

See this blog post for confirmation that the status check name is "CodeQL" and not "Code Scanning results."


Following the discussion in the comments below, the docs changes needed are as follows.

Content design for docs changes

Triaging code scanning alerts in pull requests

Update the article so that it's clear to users that:

  • "Code scanning results" is a check suite
  • "CodeQL"/other tools are checks within this suite

Sections that need to be updated:

  • "About code scanning results on pull requests"
    • "If your pull request targets a protected branch..." Make sure that it's clear that within the "Code scanning results" check suite, one or more of the checks (for example, "CodeQL") might be configured as a required check.
  • "About code scanning as a pull request check"
    • Rename section to "About the Code scanning results check suite"
    • Replace the introductory sentence with something more like: "When you configure code scanning to analyze pull requests, this adds a "Code scanning results" check suite to each pull request. Within this suite, you'll see one check for each code scanning tool that is configured to run (for example, "CodeQL").
  • "Code scanning results check"
    • Rename section to "Checks in the code scanning results suite"
    • Review all references to checks and "Code scanning results". Update so that it's clear which we're talking about where. If you need an example of a check name, use "CodeQL" but make it clear that it's an example and that there might be checks in this suite with other names.
  • "Code scanning results check failures"
    • Rename section to "Failures in the code scanning results suite"
    • Review all references to checks and "Code scanning results". Update so that it's clear which we're talking about where. If you need an example of a check name, use "CodeQL" but make it clear that it's an example and that there might be checks in this suite with other names.

mario-campos avatar Sep 20 '21 13:09 mario-campos

Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

welcome[bot] avatar Sep 20 '21 13:09 welcome[bot]

👋🏻 Hi @mario-campos - thanks for raising this issue and the related PR. It's great to have your help with the docs.

In this particular case, I think the existing docs are correct. If you look at the screenshot in the docs: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/triaging-code-scanning-alerts-in-pull-requests#code-scanning-results-check, you can see that there is a check called Code scanning results and under this there is one result for each tool that was run as part of code scanning. In this example, only the CodeQL tool was run.

When I look at the blog post that you linked, I see a screenshot showing the same behavior that's described in the docs. I'm going to close this issue and the associated PR. If I've misunderstood, please get back to us.

felicitymay avatar Sep 20 '21 14:09 felicitymay

@felicitymay thank YOU for responding so quickly!

It's certainly confusing, because the screenshots you mentioned seem to call it "Code Scanning results" in the checks UI or the pull request, but when you search for it in the Settings tab of a repository, only "CodeQL" comes up, not "Code Scanning results".

Screen Shot 2021-09-20 at 11 41 19 AM

mario-campos avatar Sep 20 '21 16:09 mario-campos

Thanks for the update @mario-campos. I agree that's confusing. Would you mind bringing that up with the development team?

felicitymay avatar Sep 20 '21 17:09 felicitymay

@felicitymay, you're right the dropdown is confusing. @mario-campos, I think that drop-down is owned by the repositories team, so they are the people to bring it up with.

However, I think the terminology on that docs page is slightly incorrect, because the items in bold (like "Code scanning results") are not "checks". The checks are the individual items nested below them. This is not obvious from most demo setups where there is only one tool (so the number of groupings and checks is the same), but if you look at this example you will see that the check count on the tab (24) is referring to the nested items, not the groupings in bold.

I think the customer-facing name for the groupings is "check suites", but I'm not 100% sure about that.

So, for example, I think that...

For all configurations of code scanning, the check that contains the results of code scanning is: Code scanning results. The results for each analysis tool used are shown separately.

...should read something like...

For all configurations of code scanning, the check suite that contains the results of code scanning is: Code scanning results. There is a separate check in this suite for each analysis tool.

tibbes avatar Sep 21 '21 17:09 tibbes

Many thanks @mario-campos for investigating further and @tibbes for the clarification. I'll reopen the issue.

It sounds as if we should still talk about Code scanning results since it's the only text we know absolutely will be present, but that we need to describe it as a "check suite" as you suggest. It sounds as if it would be worth mentioning CodeQL as an example of the check you see in this suite if you run CodeQL analysis.

felicitymay avatar Sep 21 '21 17:09 felicitymay

@felicitymay, I think @tibbes made a good point that will help add some clarity around the wording.

My original point behind the issue/PR is that it's not clear what the user should be looking for. The docs mention "Code Scanning results" but that text doesn't show up in the settings UI; only "CodeQL" shows up. So, I feel that the docs should also be re-worded somehow around what the user should expect to see and what the user should do.

mario-campos avatar Sep 21 '21 19:09 mario-campos

It sounds as if we should still talk about Code scanning results since it's the only text we know absolutely will be present

Unfortunately that's the difficulty here. When the user turns on "Required status checks" they have to list the checks that are required. And as the screenshot above shows, the check names are listed without the name of the check suite. So, I'm not sure there's any way round saying something a bit clunky here.

@mario-campos, your point is true, but it's not just as simple as writing "CodeQL" instead of "Code scanning results", because users can plug any tools they like into code scanning, so there might not be any check called CodeQL.

I think it's a bug that the dropdown in the protected branches settings just lists check names on their own, because in the pull request view they are shown as <suite> / <check> like this: image It's even ambiguous, because you can create checks with the same name in different suites trivially with actions: image

tibbes avatar Sep 21 '21 19:09 tibbes

My original point behind the issue/PR is that it's not clear what the user should be looking for. The docs mention "Code Scanning results" but that text doesn't show up in the settings UI; only "CodeQL" shows up. So, I feel that the docs should also be re-worded somehow around what the user should expect to see and what the user should do.

👍🏻 I agree absolutely that the docs should explain what users see and need to do. However, please note that the article that we're discussing aims to help users understand and act on the information they are shown in pull requests. Many of the people reading this article will not have access to edit required checks.

I don't think that we have any documentation that explicitly covers how to use the settings UI to specify code scanning checks as required. I agree with @tibbes that the difficulty you highlight in the Settings UI seems more like a UI bug than anything else (although I also agree that the docs don't cover this):

I think it's a bug that the dropdown in the protected branches settings just lists check names on their own, because in the pull request view they are shown as / like this:

felicitymay avatar Sep 21 '21 20:09 felicitymay

However, please note that the article that we're discussing aims to help users understand and act on the information they are shown in pull requests. Many of the people reading this article will not have access to edit required checks.

Hmm, yeah, definitely true. Well, given this rabbit hole I've dug, it seems that there is a bigger issue here, but it's not the kind of issue that can be solved right now with a quick PR. @felicitymay, I'll defer to your best judgement on how to proceed 😃

mario-campos avatar Sep 21 '21 21:09 mario-campos

Thanks for your patience with these discussions @mario-campos 🙇🏻‍♀️

Let me know what you think of the suggested docs changes. Once we have a plan that everyone's happy with we can add it to the issue summary and anyone with the time to make the changes will be able to raise a PR to update the article.

Proposed solution

Content design for docs changes

Triaging code scanning alerts in pull requests

Update the article so that it's clear to users that:

  • "Code scanning results" is a check suite
  • "CodeQL"/other tools are checks within this suite

Sections that need to be updated:

  • "About code scanning results on pull requests"
    • "If your pull request targets a protected branch..." Make sure that it's clear that within the "Code scanning results" check suite, one or more of the checks (for example, "CodeQL") might be configured as a required check.
  • "About code scanning as a pull request check"
    • Rename section to "About the Code scanning results check suite"
    • Replace the introductory sentence with something more like: "When you configure code scanning to analyze pull requests, this adds a "Code scanning results" check suite to each pull request. Within this suite, you'll see one check for each code scanning tool that is configured to run (for example, "CodeQL").
  • "Code scanning results check"
    • Rename section to "Checks in the code scanning results suite"
    • Review all references to checks and "Code scanning results". Update so that it's clear which we're talking about where. If you need an example of a check name, use "CodeQL" but make it clear that it's an example and that there might be checks in this suite with other names.
  • "Code scanning results check failures"
    • Rename section to "Failures in the code scanning results suite"
    • Review all references to checks and "Code scanning results". Update so that it's clear which we're talking about where. If you need an example of a check name, use "CodeQL" but make it clear that it's an example and that there might be checks in this suite with other names.

UI improvement

Raise an issue with the appropriate team to include the names of check suites in the UI for setting branch protection. Otherwise, it's difficult for people to identify the check they want - especially since it's possible to run checks with the same name in different suites (as demonstrated by @tibbes).

felicitymay avatar Sep 22 '21 12:09 felicitymay

Thanks to @mario-campos for the 👍

I've updated the issue summary with the proposed content design plan.

felicitymay avatar Oct 04 '21 13:10 felicitymay