Corrections for Fix bootstraps for webview and service_only (recently merged)
#1586
Conversation
| @@ -687,7 +689,7 @@ def _read_configuration(): | |||
| if args.meta_data is None: | |||
| args.meta_data = [] | |||
|
|
|||
| if (not hasattr(args, 'services')) or args.services is None: | |||
| if getattr(args, 'services', None) is None: | |||
| args.services = [] | |||
There was a problem hiding this comment.
Now I'm thinking we may go even more concise with:
args.services = getattr(args, 'services', [])What do you think?
There was a problem hiding this comment.
yes that looks exactly how the default value of getattr is supposed to be used like 👍
There was a problem hiding this comment.
oh wait, actually this was supposed to also apply if args.services is None, right? which it won't, so it's not the same thing
There was a problem hiding this comment.
This would work, but it looks uglier: args.services = getattr(args, 'services', None) or [] (or alternatively, args.services = getattr(args, 'services', []) or [] but that's super redundant)
There was a problem hiding this comment.
True, the semantic would not be the same for the args.services is None use case. But is that even an use case with add_argument('--service', dest='services', action='append')? I'm really wondering
There was a problem hiding this comment.
hmm only if the default is wrong, I suppose. if we checked that the default has a proper value and never None then it would be fine
We merged by mistake the mentioned pr with some minor issues that should had been fixed in there. We solve that in here. ¡¡¡Thanks @AndreMiras!!!
¡¡¡Thanks @KeyWeeUsr!!!
opacam
force-pushed
the
minor-fix-bootstraps
branch
from
3b5da81 to
cb94ed5
Compare
|
Well, thanks to @KeyWeeUsr that noticed that the This is related with the problematic line discussed above, which I think that all the discussion started because of the I also modified the testapp_service in order that the test made in there matches one of the options explained in the python-for-android's docs to create a service (thanks again to @KeyWeeUsr 😄...who pointed me to the right direction via discord) This changes has been tested for python2 and python3 with the service's test app (the one I modify in here) Note: While testing the new app, I realised that the file |
opacam
changed the title
Fix bootstraps for webview and service_only (recently merged)Fix bootstraps for webview and service_only (recently merged)
|
Great changeset again, thanks! |
Fixed upstream: kivy/python-for-android#1586
We merged by mistake the mentioned pr #1541 with some minor issues that should had been fixed in there. We solve that in here.
¡¡¡Thanks @AndreMiras!!!