Skip to content
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

[record-minmax] Revise driver script #13820

Merged
merged 3 commits into from
Aug 29, 2024
Merged

Conversation

mhs4670go
Copy link
Contributor

This commit revises driver script for static analysis tool.

Related: #13806
ONE-DCO-1.0-Signed-off-by: seongwoo [email protected]

@mhs4670go mhs4670go added the PR/ready for review It is ready to review. Please review it. label Aug 28, 2024
@mhs4670go mhs4670go requested review from a team and jinevening August 28, 2024 12:57
seanshpark
seanshpark previously approved these changes Aug 28, 2024
Copy link
Contributor

@seanshpark seanshpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if (arser["--num_threads"])
num_threads = arser.get<int>("--num_threads");

::get_values_from<float>(arser, "--min_percentile", 1.0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return value is not used. This will not update variables.

Suggested change
::get_values_from<float>(arser, "--min_percentile", 1.0);
float min_percentile = ::get_values_from<float>(arser, "--min_percentile", 1.0);

If you want to set default value with this function, it would be possible to remove the above default value setting.

if (arser["--num_threads"])
num_threads = arser.get<int>("--num_threads");

min_percentile = ::get_values_from<float>(arser, "--min_percentile", 1.0);
Copy link
Contributor

@jinevening jinevening Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the variable declaration code? Default value is repeated.

If you want to keep that code, it would be better to change function signature.

For example, update_arg_if_possible<float>(arser, "--min_percentile", min_percentile) -> update min_percentile if possible, return void

This commit revises driver script for static analysis tool.

ONE-DCO-1.0-Signed-off-by: seongwoo <[email protected]>
Copy link
Contributor

@jinevening jinevening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@seanshpark seanshpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@seanshpark seanshpark merged commit 0a96e7b into Samsung:master Aug 29, 2024
9 checks passed
Copy link
Contributor

@shs-park shs-park left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mhs4670go mhs4670go deleted the refac_dr3 branch September 10, 2024 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants