Sitelet https://web.archive.org/web/20260310093640/https://github.com/github/codeql/pull/3796
Skip to content

C++: QLDoc for all of Instruction.qll#3796

Merged
MathiasVP merged 6 commits intogithub:masterfrom
dbartol:codeql-c-analysis-team/40/2
Jun 26, 2020
Merged

C++: QLDoc for all of Instruction.qll#3796
MathiasVP merged 6 commits intogithub:masterfrom
dbartol:codeql-c-analysis-team/40/2

Conversation

@dbartol
Copy link

@dbartol dbartol commented Jun 25, 2020

Contributes to codeql-c-analysis-team#40

I think I've now documented every class and public predicate in Instruction.qll I've tried to include detailed semantics of each instruction where appropriate.

C++: QLDoc for all of Instruction.qll
8b02f12 
I think I've now documented every class and public predicate in `Instruction.qll` I've tried to include detailed semantics of each instruction where appropriate.
@dbartol dbartol requested a review from felicitymay June 25, 2020 00:31
@dbartol dbartol requested review from a team as code owners June 25, 2020 00:31
@felicitymay
Copy link
Contributor

felicitymay commented Jun 25, 2020

Thanks for the extra documentation and for the ping. I'll pin this to the #docs-dsp channel and one of the @github/product-docs-dsp team will review it 😄

@felicitymay felicitymay requested review from a team and removed request for felicitymay June 25, 2020 09:10
@geoffw0
Copy link
Contributor

geoffw0 commented Jun 25, 2020

I'm currently reviewing this from a code point of view, and will hopefully finish my review this afternoon.

@geoffw0
Copy link
Contributor

geoffw0 commented Jun 25, 2020

For the benefit of reviewers who are not familiar, all five copies of Instruction.qll are identical, and I believe this is enforced by a PR check. So there's no need to review more than one of the copies.

hubwriter
hubwriter previously approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@hubwriter hubwriter left a comment

Choose a reason for hiding this comment

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

looks good - just a couple of typos

@dbartol dbartol requested a review from a team as a code owner June 25, 2020 11:19
geoffw0
geoffw0 reviewed Jun 25, 2020
View reviewed changes
Copy link
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.

Looks great! Naturally with so much content, I have a handful of small corrections and questions.

@geoffw0
Copy link
Contributor

geoffw0 commented Jun 25, 2020

I'm away tomorrow, but I've reviewed everything (@hubwriter has reviewed as well) and I'm happy for this to be merged once @dbartol has picked a replacement for "Gets the size of the element pointed to by the pointer, in bytes.".

@dbartol
Copy link
Author

dbartol commented Jun 25, 2020

@rdmarsh2 Can I get your quick review of the most recent commit, after which @geoffw0 say it's ready to merge?

MathiasVP
MathiasVP approved these changes Jun 26, 2020
View reviewed changes
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

Based on @geoffw0's approval (and the changes in the last commit) I think this LGTM!

@MathiasVP MathiasVP merged commit beb6629 into github:master Jun 26, 2020
@MathiasVP MathiasVP mentioned this pull request Jun 30, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@geoffw0 geoffw0 geoffw0 left review comments

@MathiasVP MathiasVP MathiasVP approved these changes

@hubwriter hubwriter hubwriter left review comments

Assignees

@geoffw0 geoffw0

Labels

C++ documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dbartol @felicitymay @geoffw0 @MathiasVP @hubwriter