Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing issue 21741: Modifying reference documentation #23238

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pat-jpnk
Copy link

@pat-jpnk pat-jpnk commented Aug 8, 2024

Fixes #21741

Changing the reference documentation. Fix was first tested in private project, based on the comments contained in runfiles_src.h. Related to comments of issue #11212.

Copy link

google-cla bot commented Aug 8, 2024

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.

Copy link
Collaborator

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

Thanks for improving the docs!

@meteorcloudy Is this change safe to merge or does it need disambiguation so that it doesn't apply internally?

src/main/starlark/builtins_bzl/common/cc/attrs.bzl Outdated Show resolved Hide resolved
src/main/starlark/builtins_bzl/common/cc/attrs.bzl Outdated Show resolved Hide resolved
src/main/starlark/builtins_bzl/common/cc/attrs.bzl Outdated Show resolved Hide resolved
@iancha1992 iancha1992 added the team-Rules-CPP Issues for C++ rules label Aug 8, 2024
<pre><code class="lang-starlark">
const std::string path = devtools_build::GetDataDependencyFilepath(
Copy link
Member

Choose a reason for hiding this comment

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

This might be internal specific doc. /cc @comius

Copy link
Member

Choose a reason for hiding this comment

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

@pzembrod Can you help address this?

@davidben
Copy link

@meteorcloudy Is this change safe to merge or does it need disambiguation so that it doesn't apply internally?

FWIW, from the perspective of someone working on a project that needs to straddle both external and internal universes, it would be nice if the patterns could align a bit better. Every divergence is one more bit of friction we have to deal with when updating things.

@pat-jpnk pat-jpnk force-pushed the fix_issue_21741_cc_library_data_example branch from e96dcf6 to f9a9711 Compare August 19, 2024 21:46
@pat-jpnk pat-jpnk force-pushed the fix_issue_21741_cc_library_data_example branch from f9a9711 to 45eebf2 Compare August 23, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bazel's documentation references nonexistent devtools_build::GetDataDependencyFilepath API
5 participants