refactor(storage-core): move storage files out of @bitwarden/common #15076
Conversation
addisonbeck
force-pushed
the
storage-core-library
branch
2 times, most recently
from
1fdb0ab to
2fb1932
Compare
|
New Issues (1)Checkmarx found the following issues in this Pull Request
|
addisonbeck
force-pushed
the
storage-core-library
branch
6 times, most recently
from
473d3a7 to
fa999c5
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #15076 +/- ##
==========================================
- Coverage 36.75% 36.74% -0.01%
==========================================
Files 3201 3201
Lines 92657 92611 -46
Branches 13929 13924 -5
==========================================
- Hits 34060 34034 -26
+ Misses 57191 57172 -19
+ Partials 1406 1405 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
addisonbeck
force-pushed
the
storage-core-library
branch
from
fa999c5 to
137bb21
Compare
addisonbeck
force-pushed
the
storage-core-library
branch
2 times, most recently
from
d1aa48f to
676ef09
Compare
addisonbeck
force-pushed
the
storage-core-library
branch
from
676ef09 to
7b3006e
Compare
addisonbeck
force-pushed
the
storage-core-library
branch
from
7b3006e to
9fffbc0
Compare
|
|
||
| import { AbstractStorageService, ObservableStorageService, StorageUpdate } from "./storage.service"; | ||
|
|
||
| export class SerializedMemoryStorageService |
There was a problem hiding this comment.
We had two MemoryStorageServices in different places. They are nearly identical, but one worked with serialized data and the other didn't so I renamed it. This might not be the best name. These services may even be redundant.
|
| StorageUpdateType, | ||
| StorageUpdate, | ||
| ObservableStorageService, | ||
| AbstractStorageService, |
There was a problem hiding this comment.
I'm thinking this might be a good opportunity for getting rid of this prefix. We could rename it to StorageService in the storage-core lib but still re-export it as AbstractStorageService for compat.
There was a problem hiding this comment.
This was previously in a spec folder, in your opinion, should that continue?
|
|
||
| import { AbstractStorageService, ObservableStorageService, StorageUpdate } from "./storage.service"; | ||
|
|
||
| export class SerializedMemoryStorageService |
There was a problem hiding this comment.
I like that this is part of the template, to help people get a start with a test but once other tests are added do you think people should keep this test. Or should it be kept and test that the things you're expecting to export are actually there?
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-22400
📔 Objective
This PR creates a platform owned
storage-corelibrary and attempts to extract all relevant code for it from@bitwarden/common.Currently all classes are re-exported from their original homes in
@bitwarden/common. Updates to references will come on followup PRs scoped to each impacted team.This library has no dependencies on other Bitwarden code.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes