[Encrypt Bookmarks] Clear old encrypted data [chromium/src : main]
Ari Chivukula (Gerrit)
Ari Chivukula voted and added 3 comments![]( "Open in Gerrit")
Votes added by Ari Chivukula
| Auto-Submit | +1 |
| Commit-Queue | +1 |
3 comments
[Encrypt Bookmarks] Clear Old Encrypted DataAri ChivukulaNit: `Clear old encrypted data`, no need to capitalize words in a sentence.
Done
base::OnceCallback<void()> cleanup);Ari ChivukulaThis API is rather odd, because the caller can pass an arbitrary callback. The responsibilities to delete files correctly is also split between two classes.
I think we do either of:
a) Pass a `std::vector<base::FilePath>` of files that need to be deleted.
b) Don't involve `ModelLoader` at all.
I'll do (1) as we need the background task runner ModelLoader starts.
kOBSOLETE_EncryptedLocalOrSyncableBookmarksFileName[];Ari ChivukulaHere and below, please stick to the usual camelcase convention, i.e. `kObsoleteEncryptedLocalOrSyncableBookmarksFileName`.
I want to follow the convention of WebFeatures and highlight that these should not be used without real thought
Related details
- Mikel Astiz
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mikel Astiz (Gerrit)
Mikel Astiz voted and added 5 comments![]( "Open in Gerrit")
Votes added by Mikel Astiz
| Code-Review | +1 |
5 comments
#include "base/files/file_util.h"Nit: is this used?
const std::vector<base::FilePath>& files_to_delete);Can you please swap the order and add `callback` last, following usual conventions, as it indicates and completion of the function and everything else is the input the `Load()`.
// TODO(crbug.com/435317726): There was a rollback at one point and the files:I think you could repurpose this comment as a non-TODO comment next to the constants below, so occasional readers understand why these constants exist.
Related details
- Ari Chivukula
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ari Chivukula (Gerrit)
Ari Chivukula voted and added 4 comments![]( "Open in Gerrit")
Votes added by Ari Chivukula
| Auto-Submit | +1 |
| Commit-Queue | +2 |
4 comments
#include "base/files/file_util.h"Ari ChivukulaNit: is this used?
Done
Can you please swap the order and add `callback` last, following usual conventions, as it indicates and completion of the function and everything else is the input the `Load()`.
Done
// TODO(crbug.com/435317726): There was a rollback at one point and the files:I think you could repurpose this comment as a non-TODO comment next to the constants below, so occasional readers understand why these constants exist.
I moved it to the header
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |