-
Notifications
You must be signed in to change notification settings - Fork 3k
Improve importer.py #10505
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
Improve importer.py #10505
Conversation
2659a4f
to
e15068d
Compare
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.
LGTM
tools/importer/importer.py
Outdated
@@ -148,10 +153,14 @@ def get_last_cherry_pick_sha(branch): | |||
lines = output.split('\n') | |||
for line in lines: | |||
if 'cherry picked from' in line: | |||
sha = line.split(' ')[-1] | |||
return sha[:-1] | |||
sha = line.split(' ')[-1][:-1] |
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.
sha = line.split(' ')[-1][:-1] | |
# take the last line - git cherry-pick can add this line more | |
# than once if used multiple times | |
sha = line.split(' ')[-1][:-1] |
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.
switched to regex
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.
ok
tools/importer/importer.py
Outdated
# Set logging level | ||
logging.basicConfig(level=level) | ||
rel_log = logging.getLogger("Importer") | ||
|
||
if (args.repo_path is None) or (args.config_file is None): | ||
rel_log.error("Repository path and config file required as input. Use \"--help\" for more info.") | ||
exit(1) | ||
rel_log.error( |
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.
this can be replaced by required=True
in parser
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.
ok
tools/importer/importer.py
Outdated
|
||
repo = abspath(args.repo_path) | ||
if not os.path.exists(repo): | ||
rel_log.error("%s not found.", args.repo_path) | ||
exit(1) | ||
sys.exit(1) |
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.
use parser.error
or add an action to validate it in the parser
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.
ok
tools/importer/importer.py
Outdated
|
||
json_file = abspath(args.config_file) | ||
if not os.path.isfile(json_file): | ||
rel_log.error("%s not found.", args.config_file) | ||
exit(1) | ||
sys.exit(1) |
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.
use parser.error or add an action to validate it in the parser
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.
ok
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.
see above
744212c
to
d1fa7bd
Compare
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.
Nice clean up!
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.
lgtm
CI started |
Test run: FAILEDSummary: 2 of 11 test jobs failed Failed test jobs:
|
CI was restarted, all green now |
Description
Improve python logic and fix an issue when multiple cherry-pick messages has been added
Pull request type
Reviewers
@ARMmbed/mbed-os-tools @alzix
Release Notes