Skip to content

LGTM deprecation: Update QL detective tutorials #11502

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 4 commits into from
Dec 5, 2022

Conversation

felicitymay
Copy link
Contributor

@felicitymay felicitymay commented Nov 30, 2022

These changes remove the reliance of the tutorials on LGTM.com's query console.

Internal PR raised to fix the check failure.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@felicitymay felicitymay marked this pull request as ready for review November 30, 2022 15:54
@felicitymay felicitymay requested a review from jf205 November 30, 2022 15:54
@felicitymay
Copy link
Contributor Author

Most of the code scanning alerts were about QLDoc style issues, which are not relevant for these example queries. I've dismissed those. However, there are some that suggest improvements to the queries. I'm not equipped to judge whether these alerts indicate action that we should take to improve the queries, or whether the queries are better suited to help people understand syntax as they currently are.

jf205
jf205 previously approved these changes Dec 1, 2022
Copy link
Contributor

@jf205 jf205 left a comment

Choose a reason for hiding this comment

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

Thanks @felicitymay. Having the answers at the bottom of the article seems to work quite well. Approved pending a few updates to the 'Alternative solutions' section.

@felicitymay felicitymay changed the base branch from main to rc/3.8 December 1, 2022 17:27
@felicitymay felicitymay dismissed jf205’s stale review December 1, 2022 17:27

The base branch was changed.

Co-authored-by: James Fletcher <42464962+jf205@users.noreply.github.com>
@felicitymay
Copy link
Contributor Author

Thanks for the review @jf205. Are we happy to leave the remaining CodeQL alerts for now, or is there someone who might have time to have a look?

@felicitymay felicitymay changed the base branch from rc/3.8 to codeql-cli-2.11.5 December 2, 2022 09:39
Copy link
Contributor

@SiaraMist SiaraMist left a comment

Choose a reason for hiding this comment

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

These updates look great! I also appreciated having these as examples for my own related work on this. ✨

@felicitymay felicitymay merged commit 90c6771 into codeql-cli-2.11.5 Dec 5, 2022
@felicitymay felicitymay deleted the felicitymay-8441-detective branch December 5, 2022 09:40
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