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

chore: cleanup `len' usage #5956

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Conversation

sshedi
Copy link
Contributor

@sshedi sshedi commented Jan 9, 2025

In most cases there is need to calculate length of list of string values to do something else, just ensuring list or string is not empty is enough. This will also give a slight performance gain.

Proposed Commit Message

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@sshedi
Copy link
Contributor Author

sshedi commented Jan 22, 2025

@TheRealFalcon @blackboxsw PTAL and let me what you think of this.

@sshedi
Copy link
Contributor Author

sshedi commented Jan 30, 2025

@TheRealFalcon gentle reminder on this. If you think this is a big change for little gain, please let me know; I will close this PR.

@TheRealFalcon
Copy link
Member

@sshedi , sorry, I've been busy with some other priorities. It is a big change, but I think it's probably worthwhile. I just need to get some time to review.

@sshedi
Copy link
Contributor Author

sshedi commented Jan 31, 2025

@sshedi , sorry, I've been busy with some other priorities. It is a big change, but I think it's probably worthwhile. I just need to get some time to review.

No rush at all—I just wanted to check if this change is worth making. Sorry to bother you, and thanks!

@TheRealFalcon TheRealFalcon self-assigned this Feb 3, 2025
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Thanks @sshedi , the changes here generally look good to me. I left some inline comments

cloudinit/cmd/devel/hotplug_hook.py Outdated Show resolved Hide resolved
cloudinit/config/cc_ntp.py Outdated Show resolved Hide resolved
cloudinit/config/cc_rh_subscription.py Outdated Show resolved Hide resolved
cloudinit/cmd/devel/logs.py Outdated Show resolved Hide resolved
cloudinit/config/schema.py Outdated Show resolved Hide resolved
cloudinit/sources/DataSourceWSL.py Outdated Show resolved Hide resolved
cloudinit/netinfo.py Outdated Show resolved Hide resolved
tests/unittests/runs/test_merge_run.py Outdated Show resolved Hide resolved
tests/unittests/runs/test_simple_run.py Outdated Show resolved Hide resolved
tools/read-dependencies Outdated Show resolved Hide resolved
In most cases there is need to calculate length of list of string values
to do something else, just ensuring list or string is not empty is
enough. This will also give a slight performance gain.

Signed-off-by: Shreenidhi Shedi <shreenidhi.shedi@broadcom.com>
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @sshedi

@TheRealFalcon TheRealFalcon merged commit 56ed508 into canonical:main Feb 5, 2025
22 checks passed
@sshedi sshedi deleted the chore-fixes branch February 6, 2025 04:32
holmanb pushed a commit to holmanb/cloud-init that referenced this pull request Feb 11, 2025
In most cases there is no need to calculate length of list of string
values to do something else, just ensuring list or string is not empty
is enough. This will also give a slight performance gain.

Signed-off-by: Shreenidhi Shedi <shreenidhi.shedi@broadcom.com>
holmanb pushed a commit to holmanb/cloud-init that referenced this pull request Feb 11, 2025
In most cases there is no need to calculate length of list of string
values to do something else, just ensuring list or string is not empty
is enough. This will also give a slight performance gain.

Signed-off-by: Shreenidhi Shedi <shreenidhi.shedi@broadcom.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants