-
Notifications
You must be signed in to change notification settings - Fork 996
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
Update makefile to use pip installed dependencies #1920
Conversation
Hi @loftiskg. Thanks for your PR. I'm waiting for a feast-dev member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: Kevin Loftis <loftiskg@gmail.com>
93920b7
to
29b93a8
Compare
Codecov Report
@@ Coverage Diff @@
## master #1920 +/- ##
=======================================
Coverage 82.97% 82.97%
=======================================
Files 96 96
Lines 7376 7376
=======================================
Hits 6120 6120
Misses 1256 1256
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
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.
/lgtm
Thanks for the fix @loftiskg !
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, loftiskg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind bug |
What this PR does / why we need it:
First-time contributor here. I noticed that when I was running through the list of things in CONTRIBUTORS.md to set up my dev environment, I ran
make lint
andmake format
and a whole bunch of files were changes. Turns out that the black being used in my local setup was not the same the version of the one installed with dev dependencies. Sinceblack
is called directly as a CLI command rather than as a python module in the make command, it uses my system default rather than the one installed viapip install -e 'sdk/python[ci]'
. This would also apply to the other python linting tools utilized in the make file as well (ie mypy, isort, etc). So to ensure that the correct versions of everything are used I changed the makefile to execute black and other python installed CLI tools as python modules.Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: