Skip to content

Conversation

owen-mc
Copy link
Contributor

@owen-mc owen-mc commented Oct 10, 2025

So far I have only done python. I will do the others soon.

Also fix some mistakes in the go data flow docs that I noticed while doing this.

@owen-mc owen-mc requested a review from a team as a code owner October 10, 2025 09:39
@Copilot Copilot AI review requested due to automatic review settings October 10, 2025 09:39
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@owen-mc owen-mc added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Oct 13, 2025
Copy link
Contributor

@felicitymay felicitymay left a comment

Choose a reason for hiding this comment

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

Many thanks for your help generating the output files @owen-mc 💖

The formatting here looks generally good. I've added a few comments about improving findability and on sentence case, but very little to say on the Docs side.

sink.asIndirectExpr(1) = fc.getArgument(0) and
GetenvToGethostbynameFlow::flow(source, sink)
select getenv, fc
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend adding a link to this example to line 318, after the description of Exercise 4, to make it easier for users to find this example.

I'd suggest a similar change to all articles that have exercise numbering like this - I think that's everything down to and including JavaScript/TypeScript.

GetenvToGethostbynameFlow::flow(source, sink)
select getenv, fc
Path Query Example
Copy link
Contributor

Choose a reason for hiding this comment

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

We use sentence case:

Suggested change
Path Query Example
Path query example

Please update in all files

Path Query Example
~~~~~~~~~~~~~~~~~~

Here is the first example above, converted into a path query:
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to be more explicit here, maybe:

Suggested change
Here is the first example above, converted into a path query:
Here is the network input example above, converted into a path query:

Alternatively, you could give the two queries above a heading.

Path Query Example
~~~~~~~~~~~~~~~~~~

Here is the first example above, converted into a path query:
Copy link
Contributor

Choose a reason for hiding this comment

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

The same comment on the Python description applies here and to the Ruby, Rust, and Swift examples.

---------------

- `Exploring data flow with path queries <https://docs.github.com/en/code-security/codeql-for-vs-code/getting-started-with-codeql-for-vs-code/exploring-data-flow-with-path-queries>`__ in the GitHub documentation.
- `Creating path queries <https://codeql.github.com/docs/writing-codeql-queries/creating-path-queries/>`__ in the GitHub documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

We've changed docs sites from GitHub to CodeQL, so we can simplify this:

Suggested change
- `Creating path queries <https://codeql.github.com/docs/writing-codeql-queries/creating-path-queries/>`__ in the GitHub documentation.
- `Creating path queries <https://codeql.github.com/docs/writing-codeql-queries/creating-path-queries/>`__.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation ready-for-doc-review This PR requires and is ready for review from the GitHub docs team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants