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

Support for native services #2244

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

lerela
Copy link
Contributor

@lerela lerela commented Jun 25, 2020

Fixes #2243

@lerela lerela force-pushed the native_services_upstream branch from 1921654 to 336c636 Compare June 25, 2020 11:47
Copy link
Member

@tshirtman tshirtman left a comment

Choose a reason for hiding this comment

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

LGTM

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, why not 👍

@@ -619,6 +620,9 @@ def parse_args_and_make_package(args=None):
ap.add_argument('--service', dest='services', action='append', default=[],
help='Declare a new service entrypoint: '
'NAME:PATH_TO_PY[:foreground]')
ap.add_argument('--native-service', dest='native_services', action='append', default=[],
help='Declare a new native service: '
'package.name.service')
Copy link
Member

Choose a reason for hiding this comment

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

Minor: no sure why this line is broken down in two

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No specific reason, I just used the same codestyle than for the previous argument (line 620/621) for consistency.

Thanks for the merge!

@AndreMiras AndreMiras merged commit 5a94d07 into kivy:master Aug 14, 2020
@obfusk
Copy link
Contributor

obfusk commented Aug 14, 2020

Was it intentional to merge this into master instead of develop?

@tshirtman
Copy link
Member

ow, probably an overlook!

@lerela
Copy link
Contributor Author

lerela commented Aug 14, 2020

Ah, that's my mistake, GitHub didn't pick the default branch when I created the PR therefore it targeted master. Sorry about that.

@AndreMiras
Copy link
Member

ouch I completely missed that 🤦

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.

4 participants