Skip to content
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

Hack to avoid CodeQL CLI v2.12.3 #2126

Merged
merged 1 commit into from
Mar 1, 2023
Merged

Conversation

charisk
Copy link
Contributor

@charisk charisk commented Mar 1, 2023

(Paired with @robertbrignull)

CodeQL CLI v2.12.3 was released with a bug that causes the extension to fail so we force the extension to ignore it.

Raising the PR against main for now, but we might need to target a different branch.

The CHANGELOG will need to be updated.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@charisk charisk requested a review from aeisenberg March 1, 2023 16:32
@charisk charisk requested a review from a team as a code owner March 1, 2023 16:32
Comment on lines +321 to +322
extensionSpecificRelease &&
extensionSpecificRelease.name === "v2.12.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor:

Suggested change
extensionSpecificRelease &&
extensionSpecificRelease.name === "v2.12.3"
extensionSpecificRelease?.name === "v2.12.3"

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like at this point, we are guaranteed that 2.12.2 was not chosen since it is already being filtered out in this.getLatestRelease(). Is that true?

Copy link
Contributor Author

@charisk charisk Mar 1, 2023

Choose a reason for hiding this comment

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

extensionSpecificRelease is installed version though I think. So basically this is for the scenario where the user is already on v2.12.3.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean 2.12.2 or 2.12.3?

I agree it should be the case that latestRelease cannot be 2.12.3 since it should be considered incompatible and get filtered out. Even if you're already on 2.12.3 it'll report 2.12.2 as the latest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean 2.12.2 or 2.12.3?

Yes. :)

Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

Happy with the manual testing.

@aeisenberg
Copy link
Contributor

aeisenberg commented Mar 1, 2023

This should probably be cherry-picked to a branch based off of the latest release.

We are instead going to revert the relevant PRs and release from main.

@aeisenberg aeisenberg merged commit ea9dede into main Mar 1, 2023
@aeisenberg aeisenberg deleted the charisk/cli-v2.12.3-hack branch March 1, 2023 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants