fix(urma): address review feedback on Bazel docs and mock builds - #14
Open
cw20050111-prog wants to merge 5 commits into
Open
fix(urma): address review feedback on Bazel docs and mock builds#14cw20050111-prog wants to merge 5 commits into
cw20050111-prog wants to merge 5 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Follow-up for apache#3428. This PR applies the effective fixes made after the reviewed
urma_transportbranch state, targeting the reviewer feedback on URMA Bazel documentation/build behavior and mock usage.Reviewer feedback addressed:
DOWNLOAD_URMA_HEADERSis a CMake-only option and document the Bazel behavior for UMDK headers.Issue Number: Related to apache#3401 and apache#3428
What is changed and the side effects?
Changed:
src/urma/BUILD.bazelso the exported header glob works as intended.WITH_URMA_MOCK/--with-urma-mockas an explicit opt-in for the link-time mock whenliburmais unavailable.liburmaby best effort from the shared example helper, avoiding duplicate lookup inurma_performance.WITH_URMA=ONwith the explicit mock path and fix the mock for the C++14 build baseline.urma_transportPRs so this review branch shows the full CI signal.Side effects:
WITH_URMA=ONwithoutliburmanow fails fast unlessWITH_URMA_MOCK=ONis explicitly set. This avoids producing a binary that appears URMA-capable but only links the mock.Verification
git diff --check origin/urma_transport..HEADpassed..github/workflows/ci-linux.ymland.github/workflows/ci-macos.ymlsuccessfully with Ruby YAML.WITH_URMA=ON,WITH_URMA_MOCK=ON, cached UMDK headers, and local OpenSSL.sys/eventfd.h, which is Linux-only; the added Linux workflow covers this path on Ubuntu.Check List: