Base/inner functions for the database manager. This diff may be beneficial for this task https://linear.app/comm/issue/ENG-324/avoid-repetitions-in-services-code because this is the mutual code that may be put in a separate place.
Depends on D3081
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
I guess this code was tested with later diffs, but it would be nice if test plan was not just . for arc to allow creating a diff, but some explanation. Even if it's hard to test a diff in isolation (which we should aim for), we can describe which parts of the whole testing procedure specifically verified the given diff.
Comment Actions
I agree – the test plan should always be included. In my most recent stack (React Native upgrade) I had a test plan that I copy-pasted for many of the diffs, but for each diff I thought about the test plan. In many cases I specified something more specific than the copy-paste, and only used the copy-paste when it actually applied.
| services/backup/docker-server/contents/server/src/DatabaseManager.h | ||
|---|---|---|
| 56 ↗ | (On Diff #9212) | I thought we had concluded that this is an anti-pattern? We don't have to prioritize removing all of the current cases, but we should avoid introducing new ones |
| services/backup/docker-server/contents/server/src/DatabaseManager.h | ||
|---|---|---|
| 56 ↗ | (On Diff #9365) | Please do not use std::move https://linear.app/comm/issue/ENG-619/get-rid-of-stdmove-in-the-services-codebase |
Comment Actions
Sorry about not addressing the comments.
So the main problem here is the test plan. I'm not really sure what I could do as a test for this change. There are just some template functions. Once they're used in some specialized examples, it will be easier to come up with a testing plan for them.
The reason I didn't put a test plan at all is I couldn't see any other way to test this but just to check if it builds successfully. And I assumed that is concluded automatically by putting a diff up.
Will it suffice to write in the test plan something like this?
cd services yarn run-backup-serviceStill builds
Otherwise I don't really know what to put there.
| services/backup/docker-server/contents/server/src/DatabaseManager.h | ||
|---|---|---|
| 56 ↗ | (On Diff #9365) | Right, thanks. |
Comment Actions
That test plan is certainly better than nothing :)
I think some of the DynamoDB stuff would need to be tested, but hoping/assuming that gets handled down the line.