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

Tiny internal code reorganization to ease Agama development #1386

Merged
merged 3 commits into from
Aug 21, 2024

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Jul 23, 2024

agama-project/agama#1448 introduces a new approach to manage storage devices at Agama. Instead of relying on the Y2Storage proposals living in this repository, that PR introduces a new Y2Storage::AgamaProposal class living at the Agama repository.

That new proposal needs to:

  • Reuse the logic to detect shadowed volumes
  • Ensure the SpaceMaker does not clean up the attribute #forced_ptable_type for all devices just for the sake of it (it doesn't seem to be a clear need for such initialization).

All that could be fixed on the Agama side:

  • In the first case it would be relatively easy to just duplicate the code, since it's only 5 lines (including logging).
  • In the second case, the corresponding SpaceMaker method can be redefined by the Agama proposal to leave out the excessive initialization.

But this PR offers an alternative by modifying the code directly at the yast2-storage-ng repository, which may be considered cleaner.

@coveralls
Copy link

coveralls commented Jul 23, 2024

Coverage Status

coverage: 97.804%. remained the same
when pulling 6630f78 on ancorgs:agama_shared_classes
into e62c707 on yast:master.

@ancorgs ancorgs force-pushed the agama_shared_classes branch 4 times, most recently from 05d3821 to 1261f2a Compare July 24, 2024 13:42
@ancorgs ancorgs changed the title WIP: Extract shared logic to a new class to be used from Agama Tiny internal code reorganization to ease Agama development Aug 21, 2024
@ancorgs ancorgs marked this pull request as ready for review August 21, 2024 13:44
Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

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

LGTM

@ancorgs ancorgs merged commit 54c08ef into yast:master Aug 21, 2024
7 checks passed
Copy link

✅ Autosubmission job #10491232500 successfully finished
✅ Created submit request #1195128

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