Skip to content

CPP & C#: Review of qhelp (SD-4028)#2231

Merged
geoffw0 merged 1 commit intogithub:masterfrom
semmledocs-ac:newqueries-docscheck
Nov 5, 2019
Merged

CPP & C#: Review of qhelp (SD-4028)#2231
geoffw0 merged 1 commit intogithub:masterfrom
semmledocs-ac:newqueries-docscheck

Conversation

@semmledocs-ac
Copy link
Copy Markdown
Contributor

PR #2151 got merged without a review of the qhelp by a technical writer.
This PR makes changes I would have suggested on PR #2151.

PR github#2151 got merged without a review of the qhelp
by a technical writer.
The current PR makes changes I would have suggested on that PR.
@semmledocs-ac semmledocs-ac requested review from geoffw0 and jbj October 30, 2019 16:20
@semmledocs-ac semmledocs-ac requested review from a team and jf205 as code owners October 30, 2019 16:20
@semmledocs-ac semmledocs-ac removed the request for review from jf205 October 30, 2019 16:20
@semmledocs-ac
Copy link
Copy Markdown
Contributor Author

Out of interest: what's the difference between
csharp/ql/src/Security Features/CWE-321/HardcodedEncryptionKey.ql
and
csharp/ql/src/Security Features/CWE-321/HardcodedSymmetricEncryptionKey.ql ?
They both have the same @description and seem to do the same thing.

@jbj jbj removed their request for review October 31, 2019 14:41
@hvitved
Copy link
Copy Markdown
Contributor

hvitved commented Nov 4, 2019

Out of interest: what's the difference between
csharp/ql/src/Security Features/CWE-321/HardcodedEncryptionKey.ql
and
csharp/ql/src/Security Features/CWE-321/HardcodedSymmetricEncryptionKey.ql ?
They both have the same @description and seem to do the same thing.

It looks like those two queries could be combined into one.

Copy link
Copy Markdown
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

C# parts LGTM.

@hubwriter
Copy link
Copy Markdown
Contributor

@hvitved - thanks

Copy link
Copy Markdown
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

LGTM.

@geoffw0 geoffw0 merged commit 8c16b36 into github:master Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants