Skip to content

docs: describe annotations attr of pip_parse#1667

Merged
aignas merged 1 commit intobazel-contrib:mainfrom
bruno-digitbio:document-annotations-behavior
Jul 19, 2024
Merged

docs: describe annotations attr of pip_parse#1667
aignas merged 1 commit intobazel-contrib:mainfrom
bruno-digitbio:document-annotations-behavior

Conversation

@bruno-digitbio
Copy link
Copy Markdown
Contributor

@bruno-digitbio bruno-digitbio commented Jan 4, 2024

My default expectation would be that the keys to the annotations dictionary passed to pip_parse would use the normalize_name(...) convention, as is used elsewhere in the API. However, this does not appear to be the case. I originally was going to file a bug, but maybe just documenting the current behavior is enough?

For a minimal repro showing that this capitalization is indeed required, see https://github.com/bruno-digitbio/bug-rules-python-annotation-pyqt

In that repo,

$ bazel run //:test_has_comment
$ tail $(bazel info output_base)/external/pip_pyqt6/BUILD.bazel
$ tail $(bazel info output_base)/external/pip_wheel/BUILD.bazel

will both show # A comment at the bottom, as requested in the WORKSPACE file. However, if you first run

$ bazel run //:requirements.update

then the auto-generated requirements file will use lowercase for the requirement specification, breaking the desired behavior.

Please let me know if it makes more sense to just check in the docs, include this example (or something similar) as a small test, or something else. Apologies if I missed anything in the contribution guidelines!

@google-cla
Copy link
Copy Markdown

google-cla bot commented Jan 4, 2024

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

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@bruno-digitbio
Copy link
Copy Markdown
Contributor Author

bruno-digitbio commented Jan 4, 2024 •
edited
Loading

Having trouble signing the CLA, is this transient/typical or should I reach out to someone for help?

image

@rickeylev
Copy link
Copy Markdown
Collaborator

rickeylev commented Jan 4, 2024

Perhaps try again today? I didn't see any server errors when I triggered it to re-run; just the usual "person hasn't signed CLA" error. There isn't a CLA record for bruno-digitbio, so try re-submitting your CLA form.

It looks like your profile is marked pretty private ? Perhaps this requirement is what's tripping it up:

You may have Keep my email address private enabled. Without a visible email address, the CLA cannot be checked. Uncheck it and re-create the offending commit, or have your CLA point of contact add your @users.noreply.github.com address to the CLA group.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 2, 2024

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Jul 2, 2024
@bruno-digitbio bruno-digitbio force-pushed the document-annotations-behavior branch from 1a1b67e to c160c9f Compare July 6, 2024 18:19
@bruno-digitbio
Copy link
Copy Markdown
Contributor Author

bruno-digitbio commented Jul 6, 2024

Perhaps try again today?

I tried every couple of weeks and eventually gave up, but today I logged in and it magically worked!

It looks like your profile is marked pretty private ? Perhaps this requirement is what's tripping it up:

You may have Keep my email address private enabled. Without a visible email address, the CLA cannot be checked. Uncheck it and re-create the offending commit, or have your CLA point of contact add your @users.noreply.github.com address to the CLA group.

I don't think it was this specific setting, but it was probably some other similar privacy setting that I've since relaxed!

Not sure how to un-mark this as stale but I rebased it and looks like the docs update would still be valuable, but let me know if I should change it at all!

@github-actions github-actions bot removed the Can Close? Will close in 30 days if there is no new activity label Jul 6, 2024
aignas
aignas approved these changes Jul 8, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

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

Thanks!

@aignas aignas added this pull request to the merge queue Jul 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 8, 2024
@aignas aignas added this pull request to the merge queue Jul 19, 2024
Merged via the queue into bazel-contrib:main with commit 9ff6ab7 Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aignas aignas aignas approved these changes

@groodt groodt Awaiting requested review from groodt groodt is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bruno-digitbio @rickeylev @aignas