Commit 57ab90b
committed
Fix: Address review comments for List/ListAll stubs
This commit incorporates fixes based on code review feedback for the
stubbed StorageReference::List() and StorageReference::ListAll() methods
and the ListResult class implementation.
Key changes include:
1. **PIMPL Pattern for `List` and `ListAll` Methods**:
- The core logic for `List()` and `ListAll()`, including Future
creation and `ListResultInternal` PIMPL instantiation, has been
moved from common `StorageReference` code into the platform-specific
`StorageReferenceInternal` classes (Desktop, Android, iOS).
- Public `StorageReference::List()` and `StorageReference::ListAll()`
now correctly delegate to these internal methods.
2. **Doxygen Documentation**:
- Added comprehensive Doxygen comments for `ListResult::items()`,
`ListResult::prefixes()`, `ListResult::page_token()`, and
`ListResult::is_valid()` in
`storage/src/include/firebase/storage/list_result.h`, addressing
previous warnings.
3. **Method Naming Correction**:
- Renamed the static helper method
`ListResultInternalCommon::DeleteInternalPimpl` to
`ListResultInternalCommon::DeleteInternal` in
`storage/src/common/list_result.cc` and updated its call sites
for consistency.
4. **Comment Cleanup and Correction**:
- Reverted an unintentional change to a pre-existing comment in
`storage/src/common/storage_reference.cc`.
- Removed various extraneous or outdated comments from:
- `storage/src/ios/storage_reference_ios.h`
- `storage/src/android/storage_reference_android.cc`
- `storage/src/android/storage_reference_android.h`
- A previously flagged comment in `storage/src/ios/storage_reference_ios.mm`
was re-verified and found to be a necessary include with an
accurate comment after architectural changes.
These changes address the specific points raised in the code review,
improving code structure, documentation, and cleanliness.1 parent 1677c18 commit 57ab90b
3 files changed
Lines changed: 17 additions & 16 deletions
File tree
- storage/src
- android
- include/firebase/storage
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
30 | | - | |
31 | 29 | | |
32 | 30 | | |
33 | 31 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
| 27 | + | |
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
66 | 66 | | |
67 | 67 | | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
| 77 | + | |
77 | 78 | | |
78 | | - | |
79 | | - | |
80 | | - | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
81 | 84 | | |
82 | 85 | | |
83 | 86 | | |
| |||
0 commit comments