-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[microTVM][Zephyr][projectAPI] Minimize project build commands #12209
Conversation
0bfa420
to
f974ff9
Compare
|
||
if not zephyr_board: | ||
raise RuntimeError( | ||
f"Zephyr Board is not found in the {API_SERVER_DIR / CMAKELIST_FILENAME}" |
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 this message be changed to "No Zephyr board defined in the {API_SERVER_DIR / CMAKELIST_FILENAME}"
? Or "set in the ...
" ?
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.
done
Otherwise, LGTM 👍 |
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.
@gromero thanks for the review! PTAL
|
||
if not zephyr_board: | ||
raise RuntimeError( | ||
f"Zephyr Board is not found in the {API_SERVER_DIR / CMAKELIST_FILENAME}" |
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.
done
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.
@mehrdadh Thanks for addressing the comments. LGTM!
I'll just let the CI finish and give some time if somebody else wants to review it too.
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.
Since there are new changes since my last approval, I'm doing another review cycle. PTAL.
tests/micro/common/test_tvmc.py
Outdated
[pathlib.Path("./tvmc_relative_path_test"), pathlib.Path(tempfile.mkdtemp())], | ||
[ | ||
pathlib.Path("./tvmc_relative_path_test"), | ||
# pathlib.Path(tempfile.mkdtemp()) |
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.
Is that line really to be a comment?
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.
thanks for catching, fixed it
0d85909
to
30515eb
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. I'll just let the CI finish and give some time if somebody else wants to review it too.
…e#12209) * move cmake args to generate project * remove zephyr board from flash and run
This PR changes Zephyr template project to populate all configs in Cmake file and minimize the command to build the project.
Before this PR we had to do this:
After this PR:
This PR also removes some of the redundancies in passing project options like zephyr_board.
cc @alanmacd @gromero @guberti