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

Swift: Use entities in reorder directives #16425

Merged
merged 1 commit into from May 17, 2024
Merged

Conversation

cklin
Copy link
Contributor

@cklin cklin commented May 3, 2024

This PR changes reorder directives in upgrade/downgrade scripts to use proper entity type names, instead of using int as generic stand-in for entity types.

@github-actions github-actions bot added the Swift label May 3, 2024
@geoffw0 geoffw0 self-assigned this May 8, 2024
@geoffw0
Copy link
Contributor

geoffw0 commented May 8, 2024

I notice that CI has run "Upgrades Checks" on this PR. I would very much like to build confidence that these scripts are behaving correctly after the changes, without having to fiddle around with them on my own machine. Two questions:

  1. what do the upgrade checks actually do?
  2. I don't see Swift in the list of languages being checked, what's going on there?

I'll be happy to approve this and #16418 once I have some confidence of the test coverage.

@cklin cklin marked this pull request as ready for review May 13, 2024 16:24
@cklin cklin requested a review from a team as a code owner May 13, 2024 16:24
@cklin
Copy link
Contributor Author

cklin commented May 13, 2024

I notice that CI has run "Upgrades Checks" on this PR. I would very much like to build confidence that these scripts are behaving correctly after the changes, without having to fiddle around with them on my own machine. Two questions:

  1. what do the upgrade checks actually do?

The "Upgrades Checks" workflow takes every upgrade (and downgrade) script for the checked languages and (a) creates an empty database then (b) applies the script to the empty database.

What is checked: that the upgrade/downgrade script compiles successfully and passes the static validation checks implemented in the CLI.

What is not checked: that upgrade/downgrade applies successfully on a non-empty database and that the before and after databases produce the same query results (for any query).

For the changes in this PR, the static checks performed by "Upgrades Checks" should be sufficient to ensure that we did not break anything, if the workflow actually covers swift.

  1. I don't see Swift in the list of languages being checked, what's going on there?

I think that is most likely an unintentional omission. I will open a separate PR to address the issue.

@cklin cklin merged commit 1a4c07a into main May 17, 2024
18 checks passed
@cklin cklin deleted the cklin/swift-entities-reorder branch May 17, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants