Skip to content

Update docstrings for packaging rules/macros - #535

Merged
alexeagle merged 2 commits into
mainfrom
jonathon--rule-docstring-updates-sept2021
Oct 25, 2021
Merged

Update docstrings for packaging rules/macros#535
alexeagle merged 2 commits into
mainfrom
jonathon--rule-docstring-updates-sept2021

Conversation

@thundergolfer

@thundergolfer thundergolfer commented Sep 16, 2021

Copy link
Copy Markdown

Prompted by #534 and reading through the docstrings, I'm putting up these edits to clean up things.

  • Avoiding use of "import". Going way back to pip_import, I thought "import" was a misleading term. The rules need to install packages, and this involves running sometimes arbitrary on the system if a wheel isn't available. "import" also clashes with Python's import keyword. Rulesets such as rules_jvm_external use "resolve" and "fetch", which is appropriate for JVM dependencies because they are just file bundles you download, but this Python stuff you're installing.
  • Adding deprecation notices in docstrings, because Exposed docs for compile_pip_requirements #534 showed me that the generated documentation doesn't show anything for our deprecated rules and it's a bit confusing to see no detail.
  • Generally trying to bring docstring wording closer to rules_jvm_external. eg. use of "generates", "this macro runs a repository rule".

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:

Does this PR introduce a breaking change?

  • Yes
  • No

@google-cla google-cla Bot added the cla: yes label Sep 16, 2021
@thundergolfer
thundergolfer requested review from alexeagle and hrfuller and removed request for andyscott, brandjon and lberki September 16, 2021 05:08
alexeagle
alexeagle reviewed Sep 28, 2021
View reviewed changes

@alexeagle alexeagle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's not desirable that the markdown in the docs/ folder no longer matches the starlark generation if this PR lands. Someone would have to remember to run the update_docs.sh and we probably won't.

Let's do #540 so it's ergonomic and quick to keep them in sync always.

Comment thread python/pip.bzl
@alexeagle
alexeagle force-pushed the jonathon--rule-docstring-updates-sept2021 branch from 83dedb3 to c6fd4a8 Compare October 25, 2021 22:19
@google-cla

google-cla Bot commented Oct 25, 2021

Copy link
Copy Markdown

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla Bot added cla: no and removed cla: yes labels Oct 25, 2021
@alexeagle

alexeagle commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

@googlebot I consent.

@google-cla google-cla Bot added cla: yes and removed cla: no labels Oct 25, 2021
@alexeagle
alexeagle force-pushed the jonathon--rule-docstring-updates-sept2021 branch 2 times, most recently from 5daa36b to 123cf8c Compare October 25, 2021 22:50
@alexeagle
alexeagle force-pushed the jonathon--rule-docstring-updates-sept2021 branch from 182417d to e78c9eb Compare October 25, 2021 23:00
@alexeagle
alexeagle merged commit b111e56 into main Oct 25, 2021
@thundergolfer

thundergolfer commented Oct 25, 2021

Copy link
Copy Markdown
Author

Cheers 👍

@thundergolfer
thundergolfer deleted the jonathon--rule-docstring-updates-sept2021 branch June 14, 2022 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexeagle alexeagle alexeagle approved these changes
@hrfuller hrfuller Awaiting requested review from hrfuller

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thundergolfer @alexeagle