[email protected]

[hugooole]

No description provided.

[bazel-io]

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (dylib) have been updated in this PR.
Please review the changes. You can view a diff against the previous version in the "Generate module diff" check.

[hugooole]

@bazel-io skip_check unstable_url

[gemini-code-assist]

Code Review

The pull request adds a new module dylib with version 2.2.1 to the Bazel Central Registry. It includes the necessary files such as MODULE.bazel, source.json, presubmit.yml, and metadata.json. Patches are provided to add a BUILD.bazel file and modify the MODULE.bazel file. The review focuses on ensuring the PR adheres to the BCR policies, particularly the add-only requirement and the presence of required files.

[meteorcloudy]

Do we want cc_test here? googletest is never available.

[hugooole]

Add googletest as dev dependency. But googletest is not available when run test CI, Why? If I want to run test_binary in CI env, have I to make it as dependency not dev_dependency?

[meteorcloudy]

If you want, you can run the module as a root module via https://github.com/bazelbuild/bazel-central-registry/blob/main/docs/README.md#test-module (setting module_path to .) Then dev_dependency will be available.

[meteorcloudy]

Also you may want to add --process_headers_in_dependencies as a build flag otherwise the presubmit isn't testing anything since it's a header only cc_library.

See examples from BCR:

bazel-central-registry/modules/xtl/0.8.0/presubmit.yml

Lines 15 to 16 in ce10dbf

	build_flags:
	- '--process_headers_in_dependencies'

[meteorcloudy]

Why doesn't the example binary depend on dylib?

[hugooole]

dynamic_lib is in data field in cc_test, which as runtime data, not as dep. dynamic_lib itself is a simple test function, only dep c++ std, Only cc_test depend on dylib. cc_test will use dylib to load dynamic_lib.so at runtime

[meteorcloudy]

Looks great, thank you!