-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Change default uvision exporter to uvision5 #2614
Conversation
@MarceloSalazar needs a rebase @sarahmarshy review? |
uVision4 is no longer maintained by the Keil team (support was stopped long time ago). This is why we need to make the uvision exporter able to generate uVision5 projects by default (which is based on software packs). Also, adding note that the uvision4 exporter is now deprecated and will be removed in the future.
LGTM 👍 |
LGTM |
if tool == 'uvision' or tool == 'uvision5': | ||
log = path.join(project_dir, "build", "build_log.txt") | ||
if tool == 'uvision' or tool == 'uvision4': | ||
log = path.join(project_dir,"build","build_log.txt") |
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.
Could you keep the spaces that were there before?
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.
my bad, should be fixed now
/morph export-build |
Result: FAILUREYour command has finished executing! Here's what you wrote!
|
Seems like there was a change to the entry point for export that needs to be reflected in the tests and possibly documentation.
|
@sg @MarceloSalazar - That is actually just an entry point for the export build tests, as it limits the options for that switch to the three listed. This is the line responsible. |
Guys, I'm not entirely sure how this is expected to be fixed. Would you be able to help here? |
@sarahmarshy Would you mind helping @MarceloSalazar on what changes need to be made to |
@bridadan: the arguments to the build_test.py need to change in the CI to uvision from uvision5. |
Thanks @theotherjimmy, done. /morph export-build |
@bridadan @theotherjimmy No, that will still fail. I'll need to also change that line in the script. Marcelo, the line that I linked to above should become |
Let me test locally. Just a moment. |
The line in that script was changed in this PR to include uvision5 |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
|
@theotherjimmy cool, am an idiot. Good work Marcelo. LGTM |
@bridadan Why the do not merge label? |
Hang on, the pass was a false success because I was changing the CI config. Need to rerun. /morph test |
Result: ABORTEDYour command has finished executing! Here's what you wrote!
|
Just realized I used the wrong test command 😠 /morph export-build |
Result: FAILUREYour command has finished executing! Here's what you wrote!
|
IAR has existing failures, uvision is all passing |
Those IAR failures are clearly not related to this patch, and I don't think they are regressions. |
@theotherjimmy I agree! LGTM. @sarahmarshy ? |
uVision4 is no longer maintained by the Keil team (support was stopped
long time ago).
This is why we need to make the uvision exporter able to generate uVision5
projects by default (which is based on software packs).
Also, adding note that the uvision4 exporter is now deprecated and will be
removed in the future.
@0xc0170 @sg-