Skip to content
This repository was archived by the owner on Dec 15, 2023. It is now read-only.

Pass py_binary data deps through to underlying binary.#60

Closed
optimality wants to merge 1 commit intobazelbuild:masterfrom
optimality:add-data-dep
Closed

Pass py_binary data deps through to underlying binary.#60
optimality wants to merge 1 commit intobazelbuild:masterfrom
optimality:add-data-dep

Conversation

@optimality
Copy link

Also cleaning up the style a bit.

@bazel-io
Copy link
Member

bazel-io commented Oct 5, 2017

Can one of the admins verify this patch?

@googlebot
Copy link

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

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@pshields
Copy link

pshields commented Nov 3, 2017

I think this would fix an issue I ran into. It looks like without this change, the data deps to py_appengine_binary get silently dropped.

@pshields
Copy link

pshields commented Feb 4, 2018

Can someone take a look at this merge it in? It looks like a pretty trivial fix.

A workaround in the meantime is to create a separate py_library and put the data argument on that. Then include that py_library as a dep to py_appengine_binary.

@bazel-io
Copy link
Member

bazel-io commented Feb 4, 2018

Can one of the admins verify this patch?

Copy link
Contributor

@pmbethe09 pmbethe09 left a comment

Choose a reason for hiding this comment

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

passing the data deps LGTM, but the 'style fixes' are not appropriate, this is skylark not python.
https://docs.bazel.build/versions/master/skylark/bzl-style.html

Also, CLA not signed yet.

native.py_binary(
name="_py_appengine_" + name,
srcs = srcs,
srcs=srcs,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a cleanup. Skylark style has spaces around '=' which is different than python

@jo2y
Copy link
Contributor

jo2y commented Mar 11, 2018

FWIW, I noticed the lack of 'data = data' while working on #71 and my PR includes the same fix.

@optimality optimality closed this Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants