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

Persist decks #461

Merged
merged 13 commits into from
Dec 11, 2024
Merged

Persist decks #461

merged 13 commits into from
Dec 11, 2024

Conversation

kernelkind
Copy link
Member

on top of #452

@jb55
Copy link
Contributor

jb55 commented Nov 19, 2024

can you rebase and squash this into relevant commits ? it's becoming too difficult to review

@kernelkind
Copy link
Member Author

kernelkind commented Nov 19, 2024

can you rebase and squash this into relevant commits ? it's becoming too difficult to review

yes

@kernelkind kernelkind changed the base branch from full_decks_support to master November 19, 2024 16:01
@jb55
Copy link
Contributor

jb55 commented Nov 19, 2024

might need a rebase instead of merge. also looks like you merged the wrong master

@jb55
Copy link
Contributor

jb55 commented Nov 19, 2024

this rebase might be tricky... may need to squash a bunch of stuff first on the original base branch since there are so many edit commits

@jb55
Copy link
Contributor

jb55 commented Nov 19, 2024

yeah this is pretty brutal. I will try to fix

@kernelkind kernelkind marked this pull request as ready for review November 22, 2024 22:39
@kernelkind kernelkind requested a review from jb55 November 22, 2024 22:39
@jb55
Copy link
Contributor

jb55 commented Dec 2, 2024

doesn't merge cleanly @kernelkind

@alltheseas alltheseas added this to the notedeck beta milestone Dec 2, 2024
@jb55
Copy link
Contributor

jb55 commented Dec 3, 2024

squashed and rebased at:

persist-decks

but it seems to nuke my columns:

Dec02-165557
Dec02-165449

@kernelkind
Copy link
Member Author

but it seems to nuke my columns

Nuke columns generated from master? Or from columns generated in persist_decks? persist_decks uses a different serialization/deserialization method than master

@jb55
Copy link
Contributor

jb55 commented Dec 3, 2024

Nuke columns generated from master? Or from columns generated in persist_decks? persist_decks uses a different serialization/deserialization method than master

we have notedecks in the wild, we can't nuke people's desk setups

@kernelkind kernelkind marked this pull request as draft December 3, 2024 16:11
@jb55
Copy link
Contributor

jb55 commented Dec 5, 2024

just waiting on the migration step so we can merge

@kernelkind kernelkind force-pushed the persist_decks branch 3 times, most recently from 9b4cf50 to b9af5ba Compare December 6, 2024 02:54
@kernelkind
Copy link
Member Author

Done:

  • rebased on master
  • separated into small commits
  • added new serialization technique that decouples app structure from serialized structure

Left to do:

  • columns.json -> decks_cache.json migration

@jb55
Copy link
Contributor

jb55 commented Dec 8, 2024

pulled everything I could into master:

69e93b0ebfef introduce decks_cache
35613f2e7496 ConfigureDeck & EditDeck user interfaces
83fe173ba349 appearance fixes
40a96f1df220 decks structs

@kernelkind kernelkind force-pushed the persist_decks branch 4 times, most recently from 92b1c7f to b8774dc Compare December 10, 2024 03:40
@kernelkind
Copy link
Member Author

  • added metadata: Vec<String> field to SerializableDeck in src/storage/decks.rs in case we need more metadata besides an icon and a name.
  • added deserialization for columns.json which won't change when our data structures change
  • added logic for migration from old columns serialization to new decks serialization

@kernelkind kernelkind marked this pull request as ready for review December 10, 2024 20:19
@jb55 jb55 mentioned this pull request Dec 11, 2024
Signed-off-by: kernelkind <[email protected]>
@jb55 jb55 merged commit 06c9023 into damus-io:master Dec 11, 2024
7 checks passed
@jb55
Copy link
Contributor

jb55 commented Dec 11, 2024

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants