Move mutex.h header to public internal #792
Conversation
| @@ -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.
…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 |
| @@ -30,7 +30,7 @@ | |||
| #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...
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/
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.
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.hto the new fileapp/src/include/firebase/internal/mutex.h, leavingapp/src/mutex.has just an indirection to the new file. A future PR will deleteapp/src/mutex.hand 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::Mutexinto the public-internal includes directory, so that it can be used byfuture.h. See #747 for the rationale.Googlers can see b/206520921 for more details.