Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

drop tables listed in #1830 #4992

Merged
merged 28 commits into from Apr 8, 2019
Merged

Conversation

neilisfragile
Copy link
Contributor

@neilisfragile neilisfragile requested a review from a team April 2, 2019 16:10
@neilisfragile
Copy link
Contributor Author

CI will fix itself on matrix-org/sytest#598 getting merged

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

seems to be based on another pr? hard to review until it's merged

@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #4992 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           develop   #4992   +/-   ##
=======================================
  Coverage     60.7%   60.7%           
=======================================
  Files          332     332           
  Lines        34172   34172           
  Branches      5633    5633           
=======================================
  Hits         20743   20743           
+ Misses       11958   11955    -3     
- Partials      1471    1474    +3

@richvdh
Copy link
Member

richvdh commented Apr 3, 2019

move to delta 55

I'd prefer it if we didn't bump the schema version number for every single change.

@neilisfragile
Copy link
Contributor Author

Have not covered room_names and topic as referenced in #1830 (comment) because I want to see #4338 land first

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm otherwise

regex TEXT,
FOREIGN KEY(as_id) REFERENCES application_services(id)
);
/* We used to create a tables called application_services and
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/* We used to create a tables called application_services and
/* We used to create tables called application_services and

Copy link
Member

Choose a reason for hiding this comment

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

likewise in a number of other places

@richvdh
Copy link
Member

richvdh commented Apr 4, 2019

btw please remember to squash-merge this. we definitely don't want all 24 commits in the repo history.

@richvdh
Copy link
Member

richvdh commented Apr 4, 2019

(as per #synapse-dev, we could kill full_schemas/11 altogether)

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm. please please remember to squash-merge.

@neilisfragile neilisfragile merged commit 2d95168 into develop Apr 8, 2019
erikjohnston added a commit that referenced this pull request Apr 9, 2019
We need to drop tables in the correct order due to foreign table
constraints (on `application_services`), otherwise the DROP TABLE
command will fail.

Introduced in #4992.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants