Move mutex.h header to public internal - #792
Conversation
Sorry, something went wrong.
| @@ -93,4 +93,4 @@ class MutexLock { | |||
|
|
|||
There was a problem hiding this comment.
📝 Documentation issue: [10 lines up] warning: Member MutexLock(Mutex &mutex) (function) of class firebase::MutexLock is not documented.
Sorry, something went wrong.
…ectories for firebase_testing to fix a build error when including mutex.h
❌ Integration test FAILEDRequested by @dconeybe on commit 89ecf20
Add flaky tests to go/fpl-cpp-flake-tracker |
Sorry, something went wrong.
| #include "admob/src/include/firebase/admob/banner_view.h" | ||
| #include "admob/src/include/firebase/admob/types.h" | ||
| #include "app/src/assert.h" | ||
| #include "app/src/mutex.h" |
There was a problem hiding this comment.
......how about just keeping app/src/mutex.h, and having it be empty except for an include of app/src/include/firebase/internal/mutex.h? :)
Would make this PR much smaller...
Sorry, something went wrong.
There was a problem hiding this comment.
I'm somewhat averse to leaving app/src/mutex.h there as a mere indirection because then there become 2 ways to include firebase::Mutex (instead of only one). It is true that it would reduce the number of lines changed in this PR but at the cost of all future readers which have to follow the trail to eventually get to the mutex.h header that actually defines the Mutex and MutexLock classes.
If mixing the include fixes with the logistical changes is the part that is concerning, what about leaving app/src/mutex.h in there temporarily then delete it in a follow-up PR?
"There should be one-- and preferably only one --obvious way to do it."
https://www.python.org/dev/peps/pep-0020/
Sorry, something went wrong.
There was a problem hiding this comment.
If mixing the include fixes with the logistical changes is the part that is concerning, what about leaving app/src/mutex.h in there temporarily then delete it in a follow-up PR?
Yes, I like that idea.
Sorry, something went wrong.
There was a problem hiding this comment.
Done.
Sorry, something went wrong.
This will be moved to a separate PR. This reverts commit 81737b8.
…ase/internal/mutex.h
Rebase this away before creating a PR for this branch.
Move the guts of app/src/mutex.h to the new file app/src/include/firebase/internal/mutex.h, leaving app/src/mutex.h as just an indirection to the new file. A future PR will delete app/src/mutex.h and adjust all instances where it is included to point to the new file.
This is the second PR (the first one being #751) towards moving firebase::Mutex into the public-internal includes directory, so that it can be used by future.h. See #747 for the rationale.
Googlers can see b/206520921 for more details.