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

Undo comment change in old schema, document in new #1276

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

divergentdave
Copy link
Contributor

#1253 made a change to a comment in db/20230405185602_initial-schema.up.sql. Generally speaking, we should avoid this, since it affects the checksum, and sqlx would refuse to apply further migrations if it saw such a discrepancy. This PR undoes that change, and documents the new meaning of report_aggregations.prep_msg in the newly added migration instead. We could also not take this PR, and make additional manual database modifications to the staging and load test environments. Separately, we may want to start some living documentation of our schema in the docs directory, to codify an explanation of the current state of our schema, instead of relying on comments in many migration scripts.

@divergentdave divergentdave requested a review from a team as a code owner April 19, 2023 16:17
Copy link
Contributor

@tgeoghegan tgeoghegan left a comment

Choose a reason for hiding this comment

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

This sort of problem would be ruled out by addressing #1252.

@divergentdave divergentdave merged commit 11a88bb into main Apr 21, 2023
@divergentdave divergentdave deleted the david/fix-migrations branch April 21, 2023 17:19
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