The Wayback Machine - https://web.archive.org/web/20210104141203/https://github.com/bazelbuild/rules_python/pull/391
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

Remove the rules_python_external README #391

Open
wants to merge 3 commits into
base: master
from

Conversation

@thundergolfer
Copy link
Collaborator

@thundergolfer thundergolfer commented Dec 21, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

  • Does not include precompiled binaries, eg. .par files. See CONTRIBUTING.md for info
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Description

This README was carried over during the merge of rules_python_external, and it has info that's duplicating and sometimes contradictory of the documentation in the root README. I'd favour just removing it.

If there's anything in this README that you think should remain in the repo, I can pull it out and put it in the root README under "Using the packaging rules".

Does this PR introduce a breaking change?

  • Yes
  • No
@thundergolfer thundergolfer requested review from brandjon and lberki as code owners Dec 21, 2020
@google-cla google-cla bot added the cla: yes label Dec 21, 2020
@thundergolfer thundergolfer requested review from alexeagle and hrfuller Dec 21, 2020
Copy link
Collaborator

@hrfuller hrfuller left a comment

LGTM. Up to your discretion to add the chunks I commented on to the top level README

# If you need to depend on the wheel dists themselves, for instance to pass them
# to some other packaging tool, you can get a handle to them with the whl_requirement macro.
filegroup(
name = "whl_files",
data = [
whl_requirement("boto3"),
]
)
Comment on lines -77 to -84

This comment has been minimized.

@hrfuller

hrfuller Dec 21, 2020
Collaborator

Might be good to document this in the packaging rules.

This comment has been minimized.

@thundergolfer

thundergolfer Dec 22, 2020
Author Collaborator

I'll add a sub-section for it 👍

load("@rules_python_external//:defs.bzl", "pip_install")
pip_install(
name = "py_deps",
requirements = "//:requirements.txt",
# (Optional) You can provide a python interpreter (by path):
python_interpreter = "/usr/bin/python3.8",
# (Optional) Alternatively you can provide an in-build python interpreter, that is available as a Bazel target.
# This overrides `python_interpreter`.
# Note: You need to set up the interpreter target beforehand (not shown here). Please see the `example` folder for further details.
#python_interpreter_target = "@python_interpreter//:python_bin",
)
Comment on lines -51 to -61

This comment has been minimized.

@hrfuller

hrfuller Dec 21, 2020
Collaborator

This example still seems useful to document the python_interpreter[_target] arguments.

This comment has been minimized.

@thundergolfer

thundergolfer Dec 22, 2020
Author Collaborator

Agreed. It documented in the README but in words not code examples.

thundergolfer added a commit that referenced this pull request Jan 3, 2021
…external README into root README. ref: #391
…external README into root README. ref: #391
@thundergolfer thundergolfer requested a review from andyscott as a code owner Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.