Skip to content

Conversation

dconeybe
Copy link
Contributor

Move (most of) the implementation of firebase::Mutex into .cc files from being entirely in the mutex.h file.

This is the first PR towards moving firebase::Mutex into the public-internal includes directory, so that it can be used by future.h. See #747 for the rationale.

By splitting the implementation into .cc files it removes the need to pull other headers, like app/src/assert.h and app/src/log.h also into the public-internal headers folder.

Googlers can see b/206520921 for more details.

@dconeybe dconeybe added the skip-release-notes Skip release notes check label Nov 16, 2021
@dconeybe dconeybe self-assigned this Nov 16, 2021
@google-cla google-cla bot added the cla: yes label Nov 16, 2021
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Nov 16, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Nov 16, 2021
@github-actions
Copy link

github-actions bot commented Nov 16, 2021

❌  Integration test FAILED

Requested by @dconeybe on commit 428e197
Last updated: Tue Dec 14 13:24 PST 2021
View integration test log & download artifacts

Failures Configs
messaging [TEST] [FAILURE] [Android] [windows] [emulator_target]
(1 failed tests)  FirebaseMessagingTest.TestSendMessageToToken

Add flaky tests to go/fpl-cpp-flake-tracker

@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Nov 16, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Nov 16, 2021
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Nov 16, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Nov 16, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Nov 16, 2021
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Nov 17, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Nov 17, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Nov 17, 2021
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Dec 1, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Dec 1, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Dec 1, 2021
@dconeybe dconeybe requested a review from jonsimantov December 1, 2021 18:31
@dconeybe dconeybe marked this pull request as ready for review December 1, 2021 18:31
jonsimantov
jonsimantov previously approved these changes Dec 13, 2021
(void)ret;
#endif // !FIREBASE_PLATFORM_WINDOWS
}
void Acquire();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but maybe add a short comment above each one as long as we're in here.

@github-actions github-actions bot dismissed jonsimantov’s stale review December 13, 2021 19:06

🍞 Dismissed stale approval on external PR.

@jonsimantov jonsimantov self-requested a review December 13, 2021 19:25
@dconeybe
Copy link
Contributor Author

@jonsimantov Could I bother you for another approval? Your last approval got lost when I uploaded new changes.

@dconeybe dconeybe merged commit 428e197 into main Dec 14, 2021
@dconeybe dconeybe deleted the dconeybe/MutexSplitHeaderToImpl branch December 14, 2021 18:57
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests: succeeded This PR's integration tests succeeded. labels Dec 14, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Dec 14, 2021
@firebase firebase locked and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes skip-release-notes Skip release notes check tests: failed This PR's integration tests failed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants