-
Notifications
You must be signed in to change notification settings - Fork 544
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
fix compile_pip_requirements on windows #595
fix compile_pip_requirements on windows #595
Conversation
PR bazelbuild#529 updated `click` to a version which depends on `colorama` on Windows. This broke `compile_pip_requirements` on windows. This commit resolves the issue by adding colorama as a dependency.
Now that Do you know why this might not be caught by that test and if something can be done to improve the test coverage for the rule? |
It seems that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that
test_runner.py
did not detect failing tests. Please see #597
Most unfortunate. Though, having tested these changes on both macos and windows, I think they're good. Both platforms work and I think it'd be good to have this change while the test runner issues are fixed separately.
Though, I'm not a maintainer so my opinion has little value here 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
PR #529 updated
click
to a version which depends oncolorama
on Windows. This broke the generation of arequirements.txt
lock file using thecompile_pip_requirements
rule on windows.Issue Number: #529
What is the new behavior?
The generation of a
requirements.txt
lock file using thecompile_pip_requirements
rule on windows now worksDoes this PR introduce a breaking change?
Other information