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

Added missing recommendations command and cleaned up code #1975

Merged
merged 7 commits into from
Feb 16, 2020

Conversation

inclement
Copy link
Member

@inclement inclement commented Aug 31, 2019

Added this missing command, which existed in the argument parser but wasn't actually implemented.

Not sure what format we really want, but just having the command available is probably an improvement for now.

I thought about removing the argument entirely, but it does seem useful to be able to print the p4a recommendations easily.

@inclement
Copy link
Member Author

Also updated minimum python version supported to 3.6, as originally agreed in #1918 - it's my mistake that I left it at 3.4 in that PR. This minimum version is now essential since I added f-strings to the code.

Copy link
Member

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

Looking good thanks, except the linter is crying

pythonforandroid/recommendations.py Show resolved Hide resolved
pythonforandroid/recommendations.py Show resolved Hide resolved
@inclement
Copy link
Member Author

This one is also failing only on osx with the pyconfig.h error...not sure what's wrong, although it seems fairly clear this PR is not the culprit.

@inclement
Copy link
Member Author

Tests failing only due to macos issue also present on trunk.

Copy link
Member

@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!
And I got mislead in the other PR thinking it was the one breaking osx with python3.8, but definitely not then

@inclement inclement merged commit 6a7e6ff into kivy:develop Feb 16, 2020
@inclement
Copy link
Member Author

Thanks!
Looks like we'll have to do something else about this issue breaking the develop branch tests :/

@AndreMiras
Copy link
Member

For records, this got fixed recently via #2063

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