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

[#702] Improve mirroring summary #1122

Merged
merged 10 commits into from
Apr 24, 2024
Merged

Conversation

Adnilson
Copy link
Contributor

@Adnilson Adnilson commented Apr 17, 2024

Description

This PR was created to make easier the implementation of the previous one as there were many conflicts by merging the debian changes that refactored the classes used by the implementation of this feature.

These are the changes I did on my technical interview to improve the summary of the mirroring process.

Fixes #702

Other Notes

Maybe we can update some rubocop rules

Regarding spec/lib/rmt/cli/mirror_spec.rb:

  • RSpec/ExpectOutput: Use expect { ... }.to output(...).to_stdout instead of mutating $stdout.

    I was unable to test well the summary output with the output helper, so I decided to use the $stdout which makes our test more robust.

  • RSpec/MultipleExpectations and RSpec/ExampleLength were ignored because of the changes belonging to the summary output which have more information.

RSpec/ClassLength in this case there 3 more lines than the limit which is 177. Is there a reason for this number?
I prefer deep modules (with a simple interface) than shallow modules (with a complex interface) as I learned from: A Philosophy of Software Design by John Ousterhout.

@Adnilson Adnilson self-assigned this Apr 17, 2024
@Adnilson Adnilson changed the title Get 702 working after debian refactor [702] Improve mirroring summary Apr 17, 2024
@Adnilson Adnilson changed the title [702] Improve mirroring summary [#702] Improve mirroring summary Apr 17, 2024
@Adnilson Adnilson mentioned this pull request Apr 17, 2024
9 tasks
@ngetahun ngetahun self-requested a review April 17, 2024 08:44
@Adnilson Adnilson force-pushed the get_702_working_after_debian_refactor branch from e994014 to 062d8a1 Compare April 17, 2024 15:13
Copy link
Contributor

@ngetahun ngetahun left a comment

Choose a reason for hiding this comment

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

LGTM, but please merge after rmt 2.16 is released

@Adnilson Adnilson force-pushed the get_702_working_after_debian_refactor branch from 71e0615 to be401e6 Compare April 18, 2024 08:29
@Adnilson Adnilson force-pushed the get_702_working_after_debian_refactor branch from be401e6 to df5fd7b Compare April 18, 2024 08:30
@Adnilson
Copy link
Contributor Author

LGTM, but please merge after rmt 2.16 is released

How should I update package/obs/rmt-server.changes?

@Adnilson
Copy link
Contributor Author

LGTM, but please merge after rmt 2.16 is released

How should I update package/obs/rmt-server.changes?

Just merged master and updated the release to version 2.17

@Adnilson Adnilson merged commit f5a1c0d into master Apr 24, 2024
3 checks passed
@Adnilson Adnilson deleted the get_702_working_after_debian_refactor branch April 24, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

summary of mirroring process similar to smt-mirror.log
3 participants