Page MenuHomePhabricator

[web-db] create drafts table in SQLite database
ClosedPublic

Authored by kamil on Mar 21 2023, 8:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 6:53 AM
Unknown Object (File)
Thu, Nov 7, 10:47 AM
Unknown Object (File)
Wed, Nov 6, 3:15 AM
Unknown Object (File)
Wed, Nov 6, 3:15 AM
Unknown Object (File)
Mon, Nov 4, 11:11 PM
Unknown Object (File)
Sat, Nov 2, 2:53 AM
Unknown Object (File)
Sat, Nov 2, 12:29 AM
Unknown Object (File)
Sat, Nov 2, 12:29 AM
Subscribers

Details

Summary

Table for storing drafts, matching native version

Depends on D7119

Test Plan

Check if table was created, e.g. console.log(sqliteDb.exec(PRAGMA table_info(drafts);));

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Mar 22 2023, 5:34 AM
ashoat requested changes to this revision.Mar 22 2023, 2:59 PM
ashoat added inline comments.
web/database/queries/db-queries.js
19–22 ↗(On Diff #23926)

Please keep in mind that a huge intention of your work is to unify native / web databases. You are making that effort much harder here by naming the columns differently. Can you please adjust this query to exactly match what we do in native?

Please make sure to match native EXACTLY as much as possible in every way when setting up this database!! Any differences with native should be annotated and explained in the diff, and you should expect your reviewer to request changes if they notice any differences that aren't explained.

This revision now requires changes to proceed.Mar 22 2023, 2:59 PM
kamil requested review of this revision.Mar 23 2023, 5:24 AM
kamil added inline comments.
web/database/queries/db-queries.js
19–22 ↗(On Diff #23926)

Please keep in mind that a huge intention of your work is to unify native / web databases. You are making that effort much harder here by naming the columns differently. Can you please adjust this query to exactly match what we do in native?

The link leads to just a first, old migration, in the next line of code is another migration renaming the column to make the table looks exactly like I implemented here.

Plase make sure to match native EXACTLY as much as possible in every way when setting up this database!! Any differences with native should be annotated and explained in the diff, and you should expect your reviewer to request changes if they notice any differences that aren't explained.

I did expect that from reviewers, that's why I linked the source of truth (code creating a clear database) in the summary of this diff - to indicate that id no discrepancy between web and native.

I'm sorry, I completely missed that it was an old migration. My apologies for making assumptions and leaving an unnecessarily aggressive comment

This revision is now accepted and ready to land.Mar 23 2023, 12:39 PM