allow tag to be used in mbed import#873
Conversation
bridadan
left a comment
There was a problem hiding this comment.
The change seems fine to me. I'd like to double check this with @screamerbg to ensure there isn't something that's being missed by not having this check.
|
Also as note, this affects what We tested this locally and this is the behavior that we saw, so that behavior still seems to be preserved! |
bridadan
left a comment
There was a problem hiding this comment.
I did a bit more testing, and there is a change that this introduces. This allows existing .lib files in programs to be able to specify branches and tags instead of just commits, which is a change in behavior. I'd say this behavior change is outside the scope of this PR.
The existing behavior with the error was removed in this PR, so perhaps the error can just be moved to when reading .lib files?
Thanks for additional testing 👍. This is a generic change which allows the user to provide tags/branches across. if the user wants to point a .lib to a branch/tag he/she should have the flexibility to do so on the same lines of I also thought of "reproducible build" part but all the bets are off when we allow |
What is the blinky-5-12-rc2 here? I would expect to only have to specify one tag ? |
blinky-5-12-rc2 is optional positional arguments specifying the destination name where we want to import the project. |
The behavior of what .libs can have is not documented alteast https://os.mbed.com/docs/mbed-os/v5.11/tools/working-with-mbed-cli.html or in the help text. Some background can be found in #224 thanks @bridadan for providing the issue. |
|
@bridadan have made suggested changes can you re-review. if a named branch is added in .lib it will error. Its inline with current behavior have made the error message more verbose than before. |
screamerbg
left a comment
There was a problem hiding this comment.
I'm happy with the change. Please see small comment about upper case error message.
screamerbg
left a comment
There was a problem hiding this comment.
Please see @bridadan's comment about existing libraries/projects.
## Fixes * ARMmbed#870 Use input() instead of raw_input() for py3 * ARMmbed#874 Add decode call to icetea application list output ## New features * ARMmbed#873 allow tag to be used in mbed import
Fix for IOTCORE-1104
Branching strategy is changing starting 5.12 for example.
mbed importcurrently doesn't allow the user to specify branches/tags. There was a check which explicitly restricting to import tags other thanlatestortipmaybe there is some limitation.This change will have minimal impact on user workflow they can continue using the same command
mbed import https://github.com/ARMmbed/mbed-os-example-blinky#mbed-os-5.12 blinky-5-12 -vvvmbed import https://github.com/ARMmbed/mbed-os-example-blinky#mbed-os-5.12.0-rc2 blinky-5-12-rc2 -vvv@screamerbg can you please review it.
cc @theotherjimmy @bridadan @SenRamakri