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

Update buildozer.spec #370

Merged
merged 1 commit into from
Oct 6, 2017
Merged

Update buildozer.spec #370

merged 1 commit into from
Oct 6, 2017

Conversation

sandeepsajan0
Copy link
Contributor

now camera facade of plyer(kivy) works well on android also.

@sandeepsajan0
Copy link
Contributor Author

sorry, I do not understand what's going here please anyone tell me

@kiok46
Copy link
Contributor

kiok46 commented Sep 21, 2017

@sandeepsajan0 It's just a bot. To know more visit coverals.io.

Copy link
Contributor

@sumitmadhwani sumitmadhwani left a comment

Choose a reason for hiding this comment

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

I think this bug was introduced when we made python-for-android's new_toolchain as the default one.

# (list) Application requirements
requirements = plyer,kivy
# (list) Application requirements
# android is also require for run this app on android
Copy link
Contributor

Choose a reason for hiding this comment

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

Grammar mistake :P . android library is also required to run this app on Android platform might be better.

requirements = plyer,kivy
# (list) Application requirements
# android is also require for run this app on android
requirements = plyer,kivy,android
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there is need to mention android in requirements since this buildozer.spec can also be used to create iOS app. Just informing the user with the help of comment will be enough.
requirements should contain the common libraries which are required for both Android and iOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I have done it.
now it should be fine.

@sandeepsajan0
Copy link
Contributor Author

@Malverick I think it will be fine now.

@sandeepsajan0
Copy link
Contributor Author

Now, I think it should be merge, if not tell me the reason, please.

@akshayaurora
Copy link
Member

I understand what you are trying to do here, but right now this is just a comment. How about a new pr? where you actually add android to the requirements line.

Start with a new branch and do a new pr with just one commit, plus add comments on why this change was needed.

looking forward to your new pr.

@sandeepsajan0
Copy link
Contributor Author

Yes, you are right but if I actually add android in requirements then it will not work for IOS and if I add a comment for IOS then it will be the same as above because in it I had added a comment for android.

@akshayaurora
Copy link
Member

ok, then please squash the commits into one. This would help us get a clean history.

@kivy kivy deleted a comment from coveralls Sep 29, 2017
@kivy kivy deleted a comment from coveralls Sep 29, 2017
@kivy kivy deleted a comment from coveralls Sep 29, 2017
@kivy kivy deleted a comment from coveralls Sep 29, 2017
@KeyWeeUsr
Copy link
Contributor

@akshayaurora You can still change the merging type to Squash and merge and that will add only a single commit to our repo. I'm not sure if the comment in .spec file would help. Perhaps we should add it to some p4a recipe as a hard requirement or switch to pyjnius completely if possible.

@sandeepsajan0
Copy link
Contributor Author

@akshayaurora can I squash the commits of closed pull request?

@KeyWeeUsr
Copy link
Contributor

@sandeepsajan0 afaik yeah, but if you push them back to remote (git push -f), the PR might lock (happened to me long ago). Reopening.

@KeyWeeUsr KeyWeeUsr reopened this Oct 4, 2017
@KeyWeeUsr KeyWeeUsr merged commit 4c0a635 into kivy:master Oct 6, 2017
@KeyWeeUsr
Copy link
Contributor

Thanks! :)

@kivy kivy deleted a comment from coveralls Oct 6, 2017
@kivy kivy deleted a comment from coveralls Oct 6, 2017
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.

5 participants