Update.py: New feature - update a branch instead of a fork, plus general improvements. #4093
Conversation
Added the ability to specify a branch to update rather than a fork Replaced print commands with the use of a logger Updated the run_cmd functions in line with previous improvements
|
@bridadan @theotherjimmy please review updates. |
| # OOB branch does not exist thus create it and then check it out | ||
| cmd = ['git', 'checkout', '-b', branch] | ||
| return_code = run_cmd(cmd) | ||
| if not return_code: |
There was a problem hiding this comment.
You've done such a nice job with handling errors all over this script, do you want to handle the failure case here as well?
There was a problem hiding this comment.
@bridadan Do you mean checking the return code for line 227 ? If so that is purposefully designed to drop through to the check on line 232
There was a problem hiding this comment.
In fact all the failures in that section are designed to drop through to 232 , so that one general error with branching covers them...
There was a problem hiding this comment.
Yes, and I totally missed that, thanks for pointing it out!
|
Could you change the PR title to say the feature instead of leaving it as "add an extra feature"? You also probably don't need "script udpated to". |
tools/test/examples/update.py
Outdated
| import os | ||
| from os.path import dirname, abspath, basename | ||
| import sys | ||
| import sys, logging |
There was a problem hiding this comment.
This is not consistent with the other imports in this module.
import sys
import loggingThere was a problem hiding this comment.
That is the 'nitiest' comment to date ! ;-)
tools/test/examples/update.py
Outdated
| """ Passes a command to the system and returns a True/False result once the | ||
| command has been executed, indicating success/failure. Commands are passed | ||
| as a list of tokens. | ||
| E.g. The command 'git remote -v' would be passed in as ['git', 'remote', '-v'] |
There was a problem hiding this comment.
We generally have a single sentence summary (SSS) 😝 at the beginning of our funcitons, followed by a newline and an expanded description.
tools/test/examples/update.py
Outdated
| exit_on_failure - If True exit the program on failure (default = False) | ||
|
|
||
| Returns: | ||
| result - True/False indicating the success/failure of the command |
There was a problem hiding this comment.
result - is redundant here.
tools/test/examples/update.py
Outdated
| exit_on_failure - If True exit the program on failure (default = False) | ||
|
|
||
| Returns: | ||
| result - True/False indicating the success/failure of the command |
There was a problem hiding this comment.
This should be named returncode right?
| # Set logging level | ||
| logging.basicConfig(level=level) | ||
|
|
||
| update_log = logging.getLogger("Update") |
There was a problem hiding this comment.
Should this be a global? have you run the script to verify that you get the correct logger when running the other functions in this file?
There was a problem hiding this comment.
Just checked and yes. This is not working like you think it is.
adbridge
changed the title
|
New feature is just as vague as add an extra feature. At least it's shorter... |
adbridge
changed the title
The function headers have been updated to follow the standard format that should be being used for tools in mbed. This is a one line summary followed by a descriptive block with more detail. Updated the handling of the main function so that the logger becomes global and thus works across all the functions. This has been tested with both the fork and branch options, and for levels INFO and DEBUG.
|
/test morph-nightly |
|
@theotherjimmy logger seems to be working globally for me now. Please do a quick re-review |
|
/morph test |
|
I think that the |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
…lwip_broadcast Release mbed OS 5.4.5 and mbed lib v142 Ports for Upcoming Targets Fixes and Changes 4059: [Silicon Labs] Rename targets ARMmbed#4059 4115: Support for Qt Creator Generic project export and associated Makefile ARMmbed#4115 3915: Feature vscode ARMmbed#3915 4205: tests: race test - add not supported for single threaded env ARMmbed#4205 4187: [NCS36510] Reduce default heap size allocated by IAR to 1/4 of RAM ARMmbed#4187 4145: test - add nanostack to examples.json file ARMmbed#4145 4093: Update.py: New feature - update a branch instead of a fork, plus general improvements. ARMmbed#4093 4225: fixed missing device_name for xDot and removed progen ARMmbed#4225 4243: Config: config header file should contain new line ARMmbed#4243 4251: Fix C++11 build error w/ u-blox EVK-ODIN-W2 ARMmbed#4251 4236: STM32 Fixed warning related to __packed redefinition ARMmbed#4236 4224: Add `mbed new .` output to export ARMmbed#4224 4190: LPC4088: Enable LWIP feature ARMmbed#4190 4136: Error when bootloader is specified but does not exist ARMmbed#4136 3881: Remove debug links to printf/exit in NDEBUG builds ARMmbed#3881 4260: Inherit Xadow M0 target from LPC11U35_501 ARMmbed#4260 4249: Add consistent button names across targets ARMmbed#4249 4254: Removed unused variable in TARGET_NXP/lpc17_emac.c ARMmbed#4254
Added the ability to specify a branch to update rather than a fork
Replaced print commands with the use of a logger
Updated the run_cmd functions in line with previous improvements
Tested locally , branch option tested during oob for 5.4.1 and standard updates tested for full 5.4.1 release.