-
Notifications
You must be signed in to change notification settings - Fork 8
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
Convert project to use poetry and pre-commit #81
Conversation
e7a84ab
to
0b14f68
Compare
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.
Looks good to me. Janos will have extra insights as to whether this has the potential to cause any problems.
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.
A future commit can leverage this work to build and publish a container that other managed plugins can build off of.
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.
There are a whole lot of changes here, before merging please make sure to run this on at least one plugin.
Manually applied.
Add various pre-commit checks which double-check aspects of the code.
Thanks, will do. Note that the commits are broken up so that the first 3 could be verified easier. The first 2 commits are simple automatic applications of If it would help, we could pull those out as separate PRs (all 3 in one PR, or in some combination of separate PRs). |
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.
Looks OK to me.
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.
👍
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.
seems good!
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.
Looks good to me. Verified as working on the kubeconfig plugin.
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.
👍
Changes introduced with this PR
We add various pre-commit checks which double-check various aspects of the code.
We perform the auto-code changes from
black
,isort
, andflake8
in separate commits to help make it easier to verify all the changes.By contributing to this repository, I agree to the contribution guidelines.