Skip to content

JS: create downgrades pack #9667

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.

Sign up for GitHub

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

Jump to bottom
Merged
aeisenberg merged 1 commit into from
Jun 22, 2022
Merged

JS: create downgrades pack #9667

aeisenberg merged 1 commit into from
Jun 22, 2022

Conversation

@nickrolfe
Copy link
Contributor

@nickrolfe nickrolfe commented Jun 22, 2022

No description provided.

@github-actions github-actions bot added the JS label Jun 22, 2022
@nickrolfe nickrolfe marked this pull request as ready for review June 22, 2022 16:35
@nickrolfe nickrolfe requested a review from a team as a code owner June 22, 2022 16:35
@nickrolfe
Copy link
Contributor Author

nickrolfe commented Jun 22, 2022

N.B. I haven't tested this at all. For now, somebody could test it by following these instructions (search for the paragraph starting "To test the downgrade script...").

@nickrolfe
Copy link
Contributor Author

nickrolfe commented Jun 22, 2022

This also includes a downgrade script for the changes introduced in #9457.

@cklin
Copy link
Contributor

cklin commented Jun 22, 2022

N.B. I haven't tested this at all. For now, somebody could test it by following these instructions (search for the paragraph starting "To test the downgrade script...").

I tested it by adding codeql_check_db_downgrades('javascript') to the build file and running target/test/check-db-downgrades-javascript. The test passed, so at least the downgrade is well-formed and applies successfully to an empty database.

Once this PR is merged and the semmle-code/ql submodule bumped, I can put up a PR to add the downgrade script check for JS and hook that up to checks-javascript-language-tests-linux.

cklin
cklin approved these changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@cklin cklin left a comment

Choose a reason for hiding this comment

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

LGTM in terms of the general structure of the downgrade pack and the downgrade script.

@aeisenberg aeisenberg merged commit 5432be7 into main Jun 22, 2022
@aeisenberg aeisenberg deleted the nickrolfe/js_downgrades branch June 22, 2022 17:30
@aeisenberg aeisenberg mentioned this pull request Jun 22, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cklin cklin cklin approved these changes

Assignees

No one assigned

Labels

JS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nickrolfe @cklin @aeisenberg