Skip to content

document module signature member defaults #12085

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

Merged
merged 2 commits into from
Feb 3, 2023

Conversation

ginsbach
Copy link
Contributor

@ginsbach ginsbach commented Feb 3, 2023

This is the last bit of shadowing work (assuming #12075 is merged), and fixes https://github.com/github/codeql-core/issues/3099.

@ginsbach ginsbach requested a review from alexet February 3, 2023 10:12
@ginsbach ginsbach requested a review from a team as a code owner February 3, 2023 10:12
@ginsbach ginsbach merged commit a639f13 into main Feb 3, 2023
@ginsbach ginsbach deleted the ginsbach/DocumentModuleSignatureMemberDefaults branch February 3, 2023 15:33
@jbj
Copy link
Contributor

jbj commented Feb 3, 2023

Should the default annotation also be mentioned in https://codeql.github.com/docs/ql-language-reference/annotations/?

@jbj
Copy link
Contributor

jbj commented Feb 3, 2023

@ginsbach
Copy link
Contributor Author

ginsbach commented Feb 3, 2023

And in https://codeql.github.com/docs/ql-language-reference/ql-language-specification/#annotations

That will have to wait for https://github.com/github/codeql-core/issues/2452. We currently don't even have the appropriate columns.

I suppose we can add it here though: https://codeql.github.com/docs/ql-language-reference/annotations/

@ginsbach
Copy link
Contributor Author

ginsbach commented Feb 3, 2023

And in https://codeql.github.com/docs/ql-language-reference/ql-language-specification/#annotations

That will have to wait for github/codeql-core#2452. We currently don't even have the appropriate columns.

I suppose we can add it here though: https://codeql.github.com/docs/ql-language-reference/annotations/

On second thought, I don't think we want to add it to the annotations section. While we implement it through the annotations mechanism internally, default is not actually introduced as an annotation to the user. I don't think it is helpful for users to think of default as an annotation - they should think of it as as a keyword (rather like signature).

Edit: @jbj and I discussed offline. My conclusion is that it would be strange to document default but not signature in the annotation sections. We should revisit the problem when adding parameterised modules to the QL Language specification. I have made a note on the issue.

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

Successfully merging this pull request may close these issues.

3 participants