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

Migrate to new CcInfo, JavaInfo, and PyInfo Starlark APIs #11375

Merged
merged 1 commit into from
May 6, 2019
Merged

Migrate to new CcInfo, JavaInfo, and PyInfo Starlark APIs #11375

merged 1 commit into from
May 6, 2019

Conversation

jamiesnape
Copy link
Contributor

@jamiesnape jamiesnape commented May 3, 2019

Fixes #11374. Relates bazelbuild/bazel#7298 and bazelbuild/bazel#7036. Immediately necessary to fix building binary packages on Mac.


This change is Reviewable

@jamiesnape
Copy link
Contributor Author

+@jwnimmer-tri for feature review.

Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers (waiting on @jwnimmer-tri)

@jwnimmer-tri
Copy link
Collaborator

+@EricCousineau-TRI for additional feature and platform review of the pybind.bzl changes please.

(I am checking on the install.bzl changes locally, and seeing some confusing results. Probably PEBKAC but I'll report back once I understand.)

Copy link
Collaborator

@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.

:lgtm: both.

(My problem was I'd assumed that bazel-bin/install worked; it doesn't work on master and probably has never worked.)

Reviewed 1 of 2 files at r1.
Reviewable status: LGTM missing from assignee EricCousineau-TRI(platform)

Copy link
Collaborator

@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.

Reviewed 1 of 2 files at r1.
Reviewable status: LGTM missing from assignee EricCousineau-TRI(platform)

Copy link
Contributor

@EricCousineau-TRI EricCousineau-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 1 of 2 files at r1.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),EricCousineau-TRI(platform)

@jwnimmer-tri jwnimmer-tri merged commit 547cdc4 into RobotLocomotion:master May 6, 2019
@jamiesnape jamiesnape deleted the new-starlark-apis branch May 22, 2019 13:08
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.

Installation rules are broken on Bazel 0.25
3 participants