Skip to content

Conversation

@danielflexion
Copy link
Contributor

@danielflexion danielflexion commented Nov 14, 2025

Following strcuture added in #4058 without using symlink for MODULE.bazel (#6440).

Adjusted according to other boost.* 1.83.0.bcr.1 targets:

  • presubmit.yml matrix.platform list and matrix.bazel
  • Dependencies:
    • platforms using version 0.0.10
    • rules_cc using version 0.0.14

Adding @bazel-io skip_check unstable_url as it doesn't have a stable url.

@google-cla
Copy link

google-cla bot commented Nov 14, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@bazel-io
Copy link
Member

Hello @vtsao-openai, @Vertexwahn, modules you maintain (boost.timer) 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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds the boost.timer module, version 1.83.0.bcr.1. The structure follows the pattern of other recent boost module additions, including the necessary MODULE.bazel, source.json, presubmit.yml, and an overlay directory with the BUILD.bazel file. The changes are generally well-structured. However, I've identified a significant maintainability concern related to the duplication of the MODULE.bazel file, which I've detailed in the comments. Addressing this will make the module easier to maintain in the future.

@danielflexion danielflexion force-pushed the boost.timer-1.83.0.bcr.1 branch 4 times, most recently from 0cd5929 to 654110a Compare November 14, 2025 13:08
@danielflexion danielflexion force-pushed the boost.timer-1.83.0.bcr.1 branch from 654110a to 53e45cd Compare November 14, 2025 13:10
@danielflexion
Copy link
Contributor Author

@bazel-io skip_check unstable_url

@bazel-io bazel-io added the skip-url-stability-check Skip the URL stability check for the PR label Nov 14, 2025
Vertexwahn
Vertexwahn previously approved these changes Nov 14, 2025
bazel-io
bazel-io previously approved these changes Nov 14, 2025
Copy link
Member

@bazel-io bazel-io left a comment

Choose a reason for hiding this comment

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

All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.

vtsao-openai
vtsao-openai previously approved these changes Nov 14, 2025
@bazel-io bazel-io dismissed stale reviews from Vertexwahn, vtsao-openai, and themself November 14, 2025 18:53

Require module maintainers' approval for newly pushed changes.

Vertexwahn
Vertexwahn previously approved these changes Nov 14, 2025
bazel-io
bazel-io previously approved these changes Nov 14, 2025
Copy link
Member

@bazel-io bazel-io left a comment

Choose a reason for hiding this comment

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

All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.

@Wyverald Wyverald added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Nov 14, 2025
…pendencies are not supporting it (boost.detail 1.83.0.bcr.1)
@bazel-io bazel-io dismissed stale reviews from Vertexwahn and themself November 18, 2025 09:27

Require module maintainers' approval for newly pushed changes.

Vertexwahn
Vertexwahn previously approved these changes Nov 18, 2025
bazel-io
bazel-io previously approved these changes Nov 18, 2025
Copy link
Member

@bazel-io bazel-io left a comment

Choose a reason for hiding this comment

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

All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.

@kotlaja
Copy link
Collaborator

kotlaja commented Nov 19, 2025

@danielflexion there is some failure with Bazel 7 on macos arm64. I'll remove my review since maintainers (@Vertexwahn) already approved (and my questions were answered, thanks) and PR will be merged once the failure is fixed 🤞

[Update]: I'll approve it due to resolving "requested changes".

@kotlaja kotlaja removed their request for review November 19, 2025 10:37
kotlaja
kotlaja previously approved these changes Nov 19, 2025
@bazel-io bazel-io dismissed stale reviews from Vertexwahn, kotlaja, and themself November 19, 2025 16:29

Require module maintainers' approval for newly pushed changes.

@danielflexion danielflexion force-pushed the boost.timer-1.83.0.bcr.1 branch 2 times, most recently from d3c0624 to 251f457 Compare November 19, 2025 18:15
@danielflexion danielflexion force-pushed the boost.timer-1.83.0.bcr.1 branch from 251f457 to 437858a Compare November 20, 2025 07:58
@danielflexion
Copy link
Contributor Author

@danielflexion there is some failure with Bazel 7 on macos arm64. I'll remove my review since maintainers (@Vertexwahn) already approved (and my questions were answered, thanks) and PR will be merged once the failure is fixed 🤞

[Update]: I'll approve it due to resolving "requested changes".

@kotlaja, @Vertexwahn thanks for the reviews. The mac arm64 error is related to boost.move dependency (See boostorg/move#57) which is included by boost.chrono and only used in 1 of the 4 unit tests but not the library itself. As a workaround, disabled that test for mac platform (437858a).

@kotlaja kotlaja merged commit d397bfd into bazelbuild:main Nov 20, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants