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.

Sign up for GitHub

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

Jump to bottom

Cherrypick add storage #30

Closed
wants to merge 9 commits into from
Closed

Cherrypick add storage #30

wants to merge 9 commits into from

Conversation

berrydenhartog
Copy link
Member

@berrydenhartog berrydenhartog commented Jun 3, 2024 •
edited by laurensWe
Loading

Description

this is a cherry-picked version of #24

Resolves #

Checklist

Please check all the boxes that apply to this pull request using "x":

  • I have tested the changes locally and verified that they work as expected.
  • I have followed the project's coding conventions and style guidelines.
  • I have rebased my branch onto the latest commit of the main branch.
  • I have squashed or reorganized my commits into logical units.
  • I have read, understood and agree to the Developer Certificate of Origin, which this project utilizes.

@berrydenhartog berrydenhartog changed the base branch from main to setup-mvp-cards June 3, 2024 13:20
berrydenhartog
berrydenhartog commented Jun 3, 2024
View reviewed changes
Copy link
Member Author

@berrydenhartog berrydenhartog left a comment

Choose a reason for hiding this comment

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

you should not have commited the database.sqlite3.test

berrydenhartog
berrydenhartog commented Jun 3, 2024
View reviewed changes
Copy link
Member Author

@berrydenhartog berrydenhartog left a comment

Choose a reason for hiding this comment

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

could you run pyright since the implementatin is not typed

berrydenhartog
berrydenhartog commented Jun 4, 2024
View reviewed changes
tad/services/storage.py Outdated Show resolved Hide resolved
tad/models/system_card.py Outdated Show resolved Hide resolved
tad/services/storage.py Show resolved Hide resolved
tests/services/test_storage.py Outdated Show resolved Hide resolved
tests/services/test_storage.py Show resolved Hide resolved
@laurensWe
Copy link
Member

@berrydenhartog can you review again, as you are the owner of the PR I can't ask a new review from you :p

@laurensWe laurensWe mentioned this pull request Jun 5, 2024
Closed
5 tasks
@uittenbroekrobbert uittenbroekrobbert force-pushed the setup-mvp-cards branch 3 times, most recently from 0f90671 to e756d61 Compare June 6, 2024 06:42
Base automatically changed from setup-mvp-cards to main June 6, 2024 08:47
@laurensWe laurensWe mentioned this pull request Jun 6, 2024
Merged
5 tasks
@laurensWe
Copy link
Member

duplicate of #32

@laurensWe laurensWe closed this Jun 6, 2024
@berrydenhartog berrydenhartog deleted the cherrypick-add-storage branch June 18, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurensWe laurensWe laurensWe left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@berrydenhartog @laurensWe @uittenbroekrobbert @ChristopherSpelt