Use declarative summary method for all sub-configs - #4739
svartkanin wants to merge 10 commits into
Conversation
|
Would you like me to review this? Is it ready? |
|
It is ready and I'd appreciate a review |
| target.chmod(stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP) | ||
|
|
||
| def as_summary(self) -> str: | ||
| def as_summary(self, level: SummaryLevel = SummaryLevel.Basic) -> str: |
There was a problem hiding this comment.
Since there is no change to the following, I do not see how SummaryLevel.Detailed is ever used:
-
master:
archinstall/archinstall/lib/global_menu.py
Line 533 in eba1c1a
-
pull request branch:
https://github.com/svartkanin/archinstall/blob/f2be09cf8a5b67d373f9f30a1879073f459d5fef/archinstall/lib/global_menu.py#L533
All the code conditional on level being SummaryLevel.Detailed in summary methods will be dead code.
|
|
||
| summary = obj.summary() | ||
| for sub_config in self.sub_cfg().values(): | ||
| summary = sub_config.summary(level) |
There was a problem hiding this comment.
The level default (SummaryLevel.Basic) can be removed from all the summary methods if only as_summary calls them. as_summary already has SummaryLevel.Basic as the default for level and passes it to the summary methods here. If this is unnecessary duplication of the default, make level required for summary methods by removing the default from their parameters.
| Basic = auto() | ||
| Detailed = auto() |
SubConfigabstract class for all sub-configuration entriessummarymethod which is used to assemble the installation preview