Skip to content

LGTM deprecation: Update basic queries to use VS Code #11468

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 11 commits into from
Dec 2, 2022

Conversation

felicitymay
Copy link
Contributor

@felicitymay felicitymay commented Nov 29, 2022

This pull request updates the basic query introduction articles to use VS Code in pace of LGTM.com's query console. This is a replacement for: #11423.

Please tick the list below to confirm that your team is happy with the changes.

  • C/C++ article
  • C# article
  • Go article
  • Java article
  • JavaScript article
  • Python article
  • Ruby article

The general approach was reviewed and agreed in: #11423.

For an HTML preview of the resulting docs, look at the Docs - Generate Sphinx check. You should find an artefact that you can download, so you can view the HTML generated from the rst files.

@felicitymay felicitymay marked this pull request as ready for review November 29, 2022 15:54
@felicitymay felicitymay requested review from a team November 29, 2022 15:55
jketema
jketema previously approved these changes Nov 30, 2022
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

C++ generally looks good to me. One question.

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

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Java LGTM 👍

Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Actually, one minor thing.

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>
@aibaars
Copy link
Contributor

aibaars commented Dec 1, 2022

Ruby 👍

Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

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

Go 👍

Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

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

JS 👍

Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

Generally quite nice, only one typo.
I wonder if we should find a smaller DB than saltstack/salt, but on the other hand it is nice to see a bunch of results and then a small bunch of results after refining the query..

@felicitymay felicitymay changed the base branch from rc/3.8 to codeql-cli-2.11.5 December 2, 2022 09:40
Co-authored-by: Michael B. Gale <mbg@github.com>
@felicitymay
Copy link
Contributor Author

👋🏻 I believe that I've applied all changes. I'm planning to merge this PR before I stop work at the end of my (UK) day.

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.

LGTM! 🚀

@felicitymay felicitymay merged commit 2b24870 into codeql-cli-2.11.5 Dec 2, 2022
@felicitymay felicitymay deleted the felicitymay-8441-basic-query-2 branch December 2, 2022 23:47
@yoff
Copy link
Contributor

yoff commented Dec 2, 2022

👋🏻 I believe that I've applied all changes. I'm planning to merge this PR before I stop work at the end of my (UK) day.

I checked a few times, but you must have done it later. What I would normally expect is that you re-request a review from me (in the top right reviewers panel) and then I get notified and can approve your changes. Then you do not have to merge in the "changes requested" state :-)

@felicitymay
Copy link
Contributor Author

I checked a few times, but you must have done it later. What I would normally expect is that you re-request a review from me (in the top right reviewers panel) and then I get notified and can approve your changes. Then you do not have to merge in the "changes requested" state :-)

Hi @yoff. Thanks for letting me know the expected etiquette for this situation, it's much appreciated. We rarely use that feature in the repositories I usually work in and I overlooked it. If there are changes that I missed, or you have feedback on the changes I made, please do go ahead and add them here. I'd be happy to open a new PR with any corrections.

@yoff
Copy link
Contributor

yoff commented Dec 5, 2022

If there are changes that I missed, or you have feedback on the changes I made, please do go ahead and add them here. I'd be happy to open a new PR with any corrections.

I think all my comments are addressed appropriately 👍

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.

10 participants