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

common/yaml: Mark private to placate our install #10892

Merged
merged 1 commit into from
Mar 12, 2019

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Mar 12, 2019

Adding a new header dependency involves annotating various release meta-data; we'll defer that to future work, to unbreak macOS CI.

Closes #10893. Amends #10870. Relates #10869.


This change is Reviewable

Adding a new header dependency involves annotating various release
meta-data; we'll defer that to future work, to unbreak macOS CI.
@jwnimmer-tri
Copy link
Collaborator Author

+@sammy-tri for all review, please. I've tested on macsim that this fixes the problem.

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1.
Reviewable status: all discussions resolved, LGTM missing from assignee sammy-tri, platform LGTM from [ggould-tri] (waiting on @sammy-tri)

@jwnimmer-tri jwnimmer-tri assigned ggould-tri and unassigned sammy-tri Mar 12, 2019
Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, LGTM missing from assignee sammy-tri, platform LGTM from [ggould-tri] (waiting on @jwnimmer-tri and @sammy-tri)


common/yaml/dev/BUILD.bazel, line 16 at r1 (raw file):

drake_cc_package_library(
    name = "dev",
    visibility = ["//visibility:private"],

minor: Why is this needed if the default is now private?

Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: all discussions resolved, LGTM missing from assignee sammy-tri, platform LGTM from [ggould-tri] (waiting on @sammy-tri)


common/yaml/dev/BUILD.bazel, line 16 at r1 (raw file):

Previously, ggould-tri wrote…

minor: Why is this needed if the default is now private?

OK for better or worse, package_library uses default=public right now, ignoring the BUILD file's default.

@jwnimmer-tri jwnimmer-tri merged commit 77a95f4 into RobotLocomotion:master Mar 12, 2019
@jwnimmer-tri jwnimmer-tri deleted the yaml-private branch March 12, 2019 15:14
@jwnimmer-tri
Copy link
Collaborator Author

(I'm happy to do a follow-up issue or PR to help clarify / de-magic the package_library public default.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants