Skip to content

Conversation

@jf205
Copy link
Contributor

@jf205 jf205 commented Aug 11, 2020

Follow up to https://github.com/github/semmle-code/pull/37473 (it fixes some more of https://github.com/github/semmle-docs/issues/218.)

This PR updates the links in the Learning CodeQL help to match the style used in the GitHub docs. I'll deal with the QL language reference in a separate PR.

Summary of changes

I've enclosed the following links in double quotes:

  • Links to other articles in the CodeQL or GitHub help.
  • Links anchors in other CodeQL/GitHub articles. I've also updated these links to use the full article title in the text, rather than the anchor name.
  • Links to anchors in the current article.
  • Links to other CodeQL/GitHub articles in further reading sections.

The following do not need to be in double quotes:

  • Inline links that don't use the the full article/anchor title in the text and also don't follow advice in the Localization prep checklist don't necessarily have to be in double quotes. I've removed a few of these, but we'll have a proper look at them as part of github/semmle-docs#44
  • Links to outside content.
  • Links in TOCs included in map topics.

@jf205 jf205 marked this pull request as ready for review August 11, 2020 19:19
@jf205 jf205 requested a review from shati-patel as a code owner August 11, 2020 19:19
Copy link
Contributor

@Marcono1234 Marcono1234 left a comment

Choose a reason for hiding this comment

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

Hopefully this review is helpful, though I am not really familiar with ReST.
Is it intended that some (but not all) closing double quotes have two space characters behind them, e.g.:
https://github.com/github/codeql/pull/4046/files/0ff10c319da2e2865115fc11a88777bf26416e81#diff-684eb72498d756bc13f287e5728e22cdR141

where v.getName().regexpMatch("[pP]ass(word|wd|)")
and value = v.getAnAssignedValue()
and value.getValue() != ""
and value.getValue() != "
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this changed by accident?


The presentations contain a number of query examples.
We recommend that you download `CodeQL for Visual Studio Code <https://help.semmle.com/codeql/codeql-for-vscode/procedures/setting-up.html>`__ and add the example database for each presentation so that you can find the bugs mentioned in the slides.
We recommend that you download `CodeQL for Visual Studio Code <https://help.semmle.com/codeql/codeql-for-vscode/procedures/setting-up.html>`__ and add the example database for each presentation so that you can find the bugs mentioned in the slides.
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this space added on purpose?

- **Path queries**: queries that describe the flow of information between a source and a sink in your code.

You can add custom queries to `custom query packs <https://lgtm.com/help/lgtm/about-queries#what-are-query-packs>`__ to analyze your projects in `LGTM <https://lgtm.com>`__, use them to analyze a database with the `CodeQL CLI <https://help.semmle.com/codeql/codeql-cli.html>`__, or you can contribute to the standard CodeQL queries in our `open source repository on GitHub <https://github.com/github/codeql>`__.
You can add custom queries to `custom query packs <https://lgtm.com/help/lgtm/about-queries#what-are-query-packs>`__ to analyze your projects in `LGTM <https://lgtm.com>`__, use them to analyze a database with the "`CodeQL CLI <https://help.semmle.com/codeql/codeql-cli.html>`__" ," or you can contribute to the standard CodeQL queries in our `open source repository on GitHub <https://github.com/github/codeql>`__.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the double quote after the comma on purpose?

- If you are contributing a query to the GitHub repository, please read the `query metadata style guide <https://github.com/github/codeql/blob/master/docs/query-metadata-style-guide.md>`__.
- If you are adding a custom query to a query pack for analysis using LGTM , see `Writing custom queries to include in LGTM analysis <https://lgtm.com/help/lgtm/writing-custom-queries>`__.
- If you are analyzing a database using the `CodeQL CLI <https://help.semmle.com/codeql/codeql-cli.html>`__, your query metadata must contain ``@kind``.
- If you are analyzing a database using the "`CodeQL CLI <https://help.semmle.com/codeql/codeql-cli.html>`__" ," your query metadata must contain ``@kind``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the double quote after the comma on purpose?


- :doc:`Metadata for CodeQL queries <query-metadata>`
- ":doc:`Metadata for CodeQL queries <query-metadata>`"

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the empty line here on purpose?

Note

Queries that are contributed to the open source repository, added to a query pack in LGTM, or used to analyze a database with the `CodeQL CLI <https://help.semmle.com/codeql/codeql-cli.html>`__ must have a query type (``@kind``) specified. The ``@kind`` property indicates how to interpret and display the results of the query analysis:
Queries that are contributed to the open source repository, added to a query pack in LGTM, or used to analyze a database with the `CodeQL CLI <https://help.semmle.com/codeql/codeql-cli.html>`__ must have a query type (``@kind``) specified. The ``@kind`` property indicates how to interpret and display the results of the query analysis:
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this space added on purpose?

==============

Query metadata is used to identify your custom queries when they are added to the GitHub repository or used in your analysis. Metadata provides information about the query's purpose, and also specifies how to interpret and display the query results. For a full list of metadata properties, see :doc:`Metadata for CodeQL queries <query-metadata>`. The exact metadata requirement depends on how you are going to run your query:
Query metadata is used to identify your custom queries when they are added to the GitHub repository or used in your analysis. Metadata provides information about the query's purpose, and also specifies how to interpret and display the query results. For a full list of metadata properties, see ":doc:`Metadata for CodeQL queries <query-metadata>`"
Copy link
Contributor

Choose a reason for hiding this comment

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

metadata>`"

This double quote seems to be redundant because the corresponding quote is already in line 44.
Though is this line wrapping (between line 43 & 44) here on purpose?

@jf205 jf205 force-pushed the link-quotes-learn-ql branch from edb2224 to 0dbe64e Compare August 12, 2020 08:38
hubwriter
hubwriter previously approved these changes Aug 12, 2020
@jf205
Copy link
Contributor Author

jf205 commented Aug 12, 2020

Hopefully this review is helpful, though I am not really familiar with ReST.
Is it intended that some (but not all) closing double quotes have two space characters behind them, e.g.:
https://github.com/github/codeql/pull/4046/files/0ff10c319da2e2865115fc11a88777bf26416e81#diff-684eb72498d756bc13f287e5728e22cdR141

Hi @Marcono1234. Thanks taking the time to review this. I think I've now addressed the various issues you spotted.

@jf205 jf205 removed the request for review from shati-patel August 12, 2020 08:49
jf205 and others added 2 commits August 12, 2020 09:53
Co-authored-by: Alistair Christie <54933897+hubwriter@users.noreply.github.com>
@jf205
Copy link
Contributor Author

jf205 commented Aug 12, 2020

Thanks for the suggestions @hubwriter. All addressed now. Please could you give this another approval?


For further information on using the configuration class in your analysis see the sections on global data flow in :doc:`Analyzing data flow in C/C++ <../cpp/dataflow>` and :doc:`Analyzing data flow in C# <../csharp/dataflow>`.
For more information on using the configuration class in your analysis see the sections on global data flow in ":doc:`Analyzing data flow in C/C++ <../cpp/dataflow>`"
and ":doc:`Analyzing data flow in C# <../csharp/dataflow>`."
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line wrapped here on purpose?

@shati-patel
Copy link
Contributor

@jf205 - I'm going to merge this PR since it looks complete. (I have an upcoming PR touching similar files and I want to avoid conflicts 🙈 )

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.

4 participants