Skip to content
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

some general code care #6

Merged
merged 10 commits into from
Apr 23, 2024
Merged

some general code care #6

merged 10 commits into from
Apr 23, 2024

Conversation

hoba87
Copy link
Contributor

@hoba87 hoba87 commented Apr 15, 2024

no functional changes, only

  • sorting imports
  • replacing deprecated distutils.version
  • loosening matplotlib and kivy requirements
  • pytest deprectation warnings only for this package

Copy link

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

I have nothing against code formatters like black or isort, but then should it not be part of the repo & CI.
That way we know what formatter was used, what ruling and we are sure things don't get messed up with on the next pull requests?

@hoba87
Copy link
Contributor Author

hoba87 commented Apr 16, 2024

I have added isort and black configuration settings and according changes of code formatting

@hoba87 hoba87 requested a review from AndreMiras April 23, 2024 11:13
@AndreMiras
Copy link

Thanks for looking this up 💪
This is more changes than I initially expected after introducing the formatter.
I don't have anything against black myself as I use for my personal projects, but I know it's a contentious topic in Kivy.
So I wanted to let the PR open a little bit to see if anyone would chime in.
Tehn I saw some of the recent commit from misl6 was actually running black (7864a8e), so I guess we're good.
After merging that PR, it could be nice to have a follow up for adding format checks from the CI

Copy link

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

LGTM

@AndreMiras AndreMiras merged commit f08698d into kivy-garden:master Apr 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants