-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Param tooling #20385
base: main
Are you sure you want to change the base?
Param tooling #20385
Conversation
A little bit of code cleanup in preparation for using srcparser to migrate the c parameter files to yaml definitions. * dataclasses * PEP 8-ish
Fixes a bug where any boolean tag that exists, whether or not the value is actually true, is recorded as true in parameter generation.
6281469
to
3639c73
Compare
@@ -269,7 +269,7 @@ def parse_yaml_serial_config(yaml_config): | |||
else: | |||
default_port_str = port_config['default'] | |||
|
|||
if default_port_str != "": | |||
if default_port_str != "" and default_port_str != 0: |
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.
Didn't you mention that this wasn't needed anymore?
@coderkalyan Thanks for this contribution! Could you resolve the merge conflict & apply the requested changes? |
@junwoo091400 once again I forgot about this PR :) Sure thing, I'll review the changes when I get a chance and rebase. |
This is the tooling part of #19489, rebased and cherry-picked so it can be reviewed and merged easily before we finish the rest of the parameter migration.
@bkueng Would appreciate a review from you, I don't have permissions to mark it on the PR itself though.
FYI @dagar