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
TIMOB-9309: BlackBerry: Convert argparse to optparse #71
TIMOB-9309: BlackBerry: Convert argparse to optparse #71
Conversation
Reviewers: DavidL, JP ChangeLog: - Convert argparse to optparse for builder.py script Test Cases: - Create sample project. - Run builder.py script with 'build' command - Make sure project successfully built. - Run builder.py script with 'run' command - Make sure project successfully ran. - Make sure script gives usage string in cases of missed or invalid arguments - Make sure script launches application when device password set in command line and simulator. - Make sure script launches application when device password set in command line and not set in simulator. - Make sure script fails to launch application when device password not set in command line and set in simulator. - Make sure script fails to launch application when invalid or incorrect ip address provided.
@@ -111,8 +118,8 @@ def error(msg): | |||
builder = Builder(args.project_path.decode('utf-8'), args.type.decode('utf-8'), bbndk) | |||
|
|||
retCode = 1 | |||
if (args.command == 'build'): | |||
if (pos_args[0] == 'build'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bring back the following instead of using pos_args[0]
parser.add_argument('command', choices=['build', 'run'], help='commands')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OptionParser doesn't support such format, it allows only options. In order not provide something like this --command before 'build' or 'run' commands we need to have them positional.
Reviewed |
Reviewers: DavidL, JP ChangeLog: - Convert argparse to optparse for builder.py script Test Cases: - Create sample project. - Run builder.py script with 'build' command - Make sure project successfully built. - Run builder.py script with 'run' command - Make sure project successfully ran. - Make sure script gives usage string in cases of missed or invalid argument - Make sure script launches application when device password set in command - Make sure script launches application when device password set in command - Make sure script fails to launch application when device password not set - Make sure script fails to launch application when invalid or incorrect ip
Updated |
@@ -80,39 +81,51 @@ def error(msg): | |||
|
|||
if __name__ == "__main__": | |||
|
|||
# Setup script usage | |||
parser = argparse.ArgumentParser(usage='<command> -t TYPE -d PROJECT_PATH -p NDK_PATH') | |||
# Setup script usage using argparse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a typo but shouldn't say argparse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Approved (with typo fixed before merge). |
# Setup script usage | ||
parser = argparse.ArgumentParser(usage='<command> -t TYPE -d PROJECT_PATH -p NDK_PATH') | ||
# Setup script usage using argparse | ||
parser = OptionParser(usage='<command: build | run> -t TYPE -d PROJECT_PATH -n NDK_PATH -i IP_ADDRESS [-p DEVICE_PASSWORD]') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-i IP_ADDRESS should be optional as it's only needed for the run command
NDK_PATH also was optional before and should still be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Reviewed |
|
||
runGroup = parser.add_option_group('Run/Deploy options') | ||
runGroup.add_option('-i', '--ip_address', help='(simulator | device) ip address', dest='ip_address') | ||
runGroup.add_option('-p', '--device_password', help='(simulator | device) protection password', dest='device_password') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change of short option name that you did in an earlier patch needs to be reverted as it break compatibility with the Studio. I thought they would code to the long options but they coded to the short ones instead.
Specifically -p needs to stay associated with --ndk_path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
…berry' into optparseBranch Conflicts: support/blackberry/builder.py
Reviewers: DavidL, JP ChangeLog: - Address comments Test Cases: - Create sample project. - Run builder.py script with 'build' command - Make sure project successfully built. - Run builder.py script with 'run' command - Make sure project successfully ran. - Make sure script gives usage string in cases of missed or invalid argument - Make sure script launches application when device password set in command - Make sure script launches application when device password set in command - Make sure script fails to launch application when device password not set - Make sure script fails to launch application when invalid or incorrect ip
Merged with latest and updated. |
Approved |
1 similar comment
Approved |
TIMOB-9309: BlackBerry: Convert argparse to optparse
Reviewers: DavidL, JP
ChangeLog:
Test Cases: