Skip to content

Conversation

@sarroutbi
Copy link
Contributor

No description provided.

@sarroutbi sarroutbi force-pushed the 202505141744-code-refactor-decrease-main-method-size branch 3 times, most recently from ae568b1 to 0bc7cd5 Compare May 14, 2025 16:11
@sarroutbi sarroutbi changed the title Refactor code: decrease main function size Refactor code: decrease main.rs file size May 14, 2025
@sarroutbi sarroutbi force-pushed the 202505141744-code-refactor-decrease-main-method-size branch from 0bc7cd5 to f423a1c Compare May 14, 2025 16:15
@sarroutbi sarroutbi marked this pull request as ready for review May 14, 2025 16:24
@sarroutbi sarroutbi requested a review from ansasaki May 14, 2025 16:24
@codecov
Copy link

codecov bot commented May 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.21%. Comparing base (0da07db) to head (9efc5bd).
Report is 1 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 64.21% <100.00%> (-0.03%) ⬇️
upstream-unit-tests 64.21% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
keylime-push-model-agent/src/main.rs 85.54% <100.00%> (-2.09%) ⬇️
keylime-push-model-agent/src/struct_filler.rs 100.00% <100.00%> (ø)
keylime-push-model-agent/src/url_selector.rs 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ansasaki
Copy link
Contributor

Sorry for being picky about the title of the PRs, but again I will suggest you to change it.
The thing is that in future when you are searching through history to understand when certain changes were introduced, the easiest is to find the PR where it landed.
I don't think this is a refactoring to reduce the size of main.rs. The reduction of the size of main.rs is only a consequence of your refactoring. To me the changes included here are about moving code to specific modules.

So, I suggest something like: "Move structure filling and URL selection related code to specific modules". Feel free to rephrase it if it is too long.

@sarroutbi sarroutbi changed the title Refactor code: decrease main.rs file size Move structure filling and URL selection related code to specific modules May 16, 2025
@sarroutbi sarroutbi force-pushed the 202505141744-code-refactor-decrease-main-method-size branch from f423a1c to 8a1d676 Compare May 16, 2025 07:16
@sarroutbi
Copy link
Contributor Author

sarroutbi commented May 16, 2025

Sorry for being picky about the title of the PRs, but again I will suggest you to change it. The thing is that in future when you are searching through history to understand when certain changes were introduced, the easiest is to find the PR where it landed. I don't think this is a refactoring to reduce the size of main.rs. The reduction of the size of main.rs is only a consequence of your refactoring. To me the changes included here are about moving code to specific modules.

So, I suggest something like: "Move structure filling and URL selection related code to specific modules". Feel free to rephrase it if it is too long.

My initial idea was decreasing the size of main.rs because it is a "code smell" (to have a high amount of LOC in a main file/method). But yes, I agree your suggestion makes it more descriptive and accurate. Thanks for it!!!

@sarroutbi
Copy link
Contributor Author

/packit retest-failed

Copy link
Contributor

@ansasaki ansasaki left a comment

Choose a reason for hiding this comment

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

LGTM!

This change aims to move structure filling and
URL selection related code to specific modules,
with a reduce in main.rs lines of code as a consequence

Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
@sarroutbi sarroutbi force-pushed the 202505141744-code-refactor-decrease-main-method-size branch from 8a1d676 to 9efc5bd Compare May 16, 2025 08:53
@sarroutbi sarroutbi merged commit 5fe1488 into keylime:master May 16, 2025
11 checks passed
@sarroutbi sarroutbi deleted the 202505141744-code-refactor-decrease-main-method-size branch May 16, 2025 10:01
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.

3 participants