-
Notifications
You must be signed in to change notification settings - Fork 3k
Modify update command to directly edit the mbed-os.lib files for each example #3528
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
Conversation
example specified in the supplied .json file, in a user specified fork. A pull request is then made from each fork to the ARMmbed master repo. New usage: python update.py <args> Where <args> are: [-c <file.json>] optional argument for example list, default example.json -U <github user with forked repos> -T <github authorisation token> tag
@bridadan please review. |
cmd = ['git', 'push', '-f', 'origin'] | ||
return_code = run_cmd(cmd) | ||
if not return_code: | ||
ret = True |
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.
Wow. Is this really the best way to do this? Maybe:
for cmd in [['git', 'remote', 'add', 'armmbed', arm_example],
['git', 'fetch', 'armmbed'],
['git', 'reset', '--hard', 'armmbed/master'],
['git', 'push', '-f', 'origin']]:
if run_cmd(cmd):
print("preparation of the fork failed!")
return False
return True
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.
yeah that looks better :)
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.
Just a few nits, please see below
print("Ignoring and moving on to the next example") | ||
os.chdir(cwd) | ||
if os.path.isfile("mbed-os.lib"): | ||
os.system("mv mbed-os.lib mbed-os.lib_bak") |
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.
mv
is not a valid command on Windows, can you use os.rename instead please?
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.
will update
"This usually inidicates that no update was made. Still " + \ | ||
"attempting to create a tag.") | ||
if not return_code: | ||
body = "Please test/merge this PR and then tag Master with " + tag |
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.
Yay for bots! 😄 🤖
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.
Until all the example owners have automatic testing of their examples for each PR, then they need the prompt!
In prepare_fork(), change nested IF to neat command loop.
@bridadan @theotherjimmy review comments added and retested locally. Please re-review. |
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
as specified in the supplied .json file, in a user specified fork.
A pull request is then made from each fork to the ARMmbed master repo.
New usage:
python update.py
Where are:
[-c <file.json>] optional argument for example list, default
example.json
-U
-T
tag