-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Query help style-guide #595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
docs/query-help-style-guide.md
Outdated
|
||
Section-level elements are used to group the information within the query help file. All query help files should include at least the following section elements, in the order specified: | ||
|
||
1. `overview`—a summary of the purpose of the query, including an explanation of what the code does and the how the behavior of the program is affected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds wrong. I think the overview section is more about giving the necessary background information to understand the issue that the query is trying to identify. Also, using a phrase such as "the code" is potentially ambiguous when used like this, as it could refer both to the QL code of the query or the code being analysed.
docs/query-help-style-guide.md
Outdated
1. `overview`—a summary of the purpose of the query, including an explanation of what the code does and the how the behavior of the program is affected. | ||
2. `recommendation`—information on how to fix the issue highlighted by the query. | ||
3. `example`—an example of code showing the problem. Where possible, this section should also include a solution to the issue. | ||
4. `references`—relevant references to best practice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just references to best practice. That's of course nice to include, if applicable. References to authoritative sources on relevant language semantics should be included, and potentially other relevant references.
docs/query-help-style-guide.md
Outdated
|
||
Whenever possible, you should include a code example that helps to explain the issue you are highlighting. Any code examples that you include should adhere to the following guidelines: | ||
|
||
* The example should be less than 20 lines and ideally it should be runnable. If this is not possible, ensure that the example very clearly illustrates the issue that you are highlighting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's important that it's runnable. For compiled languages, including enough stuff to make a snippet runnable would unnecessarily bloat the example code.
docs/query-help-style-guide.md
Outdated
<p>This example highlights poor coding practice</p> | ||
<sample language = "java" /> | ||
|
||
Example of poor code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem properly nested. There's no code in the <sample />
. Also, aren't we usually recommending to always have the example code snippets in their own files and include them with the src="file.java"
property as below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested the nested code approach after reading the 'Block elements' table in this topic: https://help.semmle.com/wiki/display/SD/Qhelp+files – and since I couldn't find any examples of this approach in the qhelp of our built-in queries the nesting is likely to be incorrect!
Happy to delete this part if specifying src
is the preferred style.
docs/query-help-style-guide.md
Outdated
|
||
## Including references | ||
|
||
You should include one or more references to provide further information about the problem that your query is designed to find. References can be of the following types: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should probably be stated that the referenced should be a list formatted with <li> .. </li>
for each item.
|
||
## Query help example | ||
|
||
The following example is a qhelp file for a query from the standard query suite for Java: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should probably be mentioned somewhere above that code words, such as keywords and the like, should be formatted with <code>..</code>
as exemplified below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline comments.
@aschackmull thanks for the review. I think I have been able to address your feedback. |
docs/query-help-style-guide.md
Outdated
1. `overview`—a short summary of the issue that the query identifies, including an explanation of how it could affect the behavior of the program. | ||
2. `recommendation`—information on how to fix the issue highlighted by the query. | ||
3. `example`—an example of code showing the problem. Where possible, this section should also include a solution to the issue. | ||
4. `references`—relevant references, such as authoritative sources on langauage semantics and best practice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: s/langauage/language/
One final comment re. a typo, otherwise LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 from me.
Add comment to `HasEllpsisTable`
This PR adds a query help style-guide to the docs folder. As with #581, the aim is to help internal/external query writers write informative qhelp which is consistent with the style adopted in the existing qhelp for the built-in queries.
@calumgrant @geoffw0 @aschackmull – thanks for your help on the metadata style-guide. I'd be very grateful for reviews here if possible please.