Skip to content

Conversation

@mutalibmohammed
Copy link

@mutalibmohammed mutalibmohammed commented Nov 27, 2025

This PR adds support to use std::format available in C++20 onwards with spdlog.

Added a use_fmt flag to allow users to use std::format instead of fmt.

@google-cla
Copy link

google-cla bot commented Nov 27, 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 @Vertexwahn, @c8ef, modules you maintain (spdlog) 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 a new version 1.16.0.bcr.2 for spdlog to introduce support for std::format via a use_fmt build flag. The changes look good and correctly implement the feature toggle using select() in the BUILD.bazel file and test both configurations in presubmit.yml. I've found a couple of minor issues in the presubmit.yml file related to formatting and consistency that could affect presubmit jobs. Please see my comments for details.

@mutalibmohammed
Copy link
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 27, 2025
Vertexwahn
Vertexwahn previously approved these changes Nov 27, 2025
bazel-io
bazel-io previously approved these changes Nov 27, 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.

@bazel-io bazel-io dismissed stale reviews from Vertexwahn and themself November 27, 2025 17:26

Require module maintainers' approval for newly pushed changes.

@meteorcloudy meteorcloudy added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Nov 28, 2025
@meteorcloudy meteorcloudy enabled auto-merge (squash) November 28, 2025 09:56
meteorcloudy
meteorcloudy previously approved these changes Nov 28, 2025
auto-merge was automatically disabled November 28, 2025 11:24

Head branch was pushed to by a user without write access

@bazel-io bazel-io dismissed meteorcloudy’s stale review November 28, 2025 11:25

Require module maintainers' approval for newly pushed changes.

@mutalibmohammed
Copy link
Author

mutalibmohammed commented Nov 28, 2025

Hi @meteorcloudy @bazelbuild/bcr-maintainers, could you have a look at why the CI is failing for the intel macos machine?

@Vertexwahn
Copy link
Contributor

Vertexwahn commented Nov 29, 2025

@mutalibmohammed I think that was just a temporary error (Error in download: java.io.IOException: Error downloading [file:/Users/buildkite/builds/bk-macos-intel-hsll/bazel/bcr-presubmit/modules/spdlog/1.16.0.bcr.2/MODULE.bazel] to /private/var/tmp/_bazel_buildkite/9d3aed0c624f300002dc845796c6aa9b/external/spdlog+/MODULE.bazel: /private/var/tmp/_bazel_buildkite/9d3aed0c624f300002dc845796c6aa9b/external/spdlog+/MODULE.bazel (Permission denied)

Please reabase your PR and push again - lets see if this happens again

@mutalibmohammed mutalibmohammed force-pushed the mutalib/spdlog/use_std_format branch from 8c0c87b to 0a5da04 Compare November 30, 2025 04:11
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.

4 participants