openSUSE / openSUSE-release-tools

Tools to aid in staging and release work for openSUSE/SUSE
GNU General Public License v2.0
57 stars 90 forks source link

pkglistgen: tool: decouple summary dict creation #3078

Closed g7 closed 1 month ago

g7 commented 1 month ago

The summary dictionary used to create summary-{scope}.txt was previously returned by solve_project() via write_all_groups().

After c46dd3e304d8e66e5f8f6a51bfa3ebda8cba8936, solve_project() doesn't return anything anymore, as write_all_groups() is being invoked just after solve_project().

Thus, the "summary" dictionary would always be None.

Since the summary dict creation is generic enough, move it to its own function, and call it afterwards after project solving.

This commit also drops the return statement both on write_all_groups() and write_productcompose() (where it would always return an empty dictionary), and moves the write_productcompose() call after solve_project() as well to match the change with write_all_groups().

Vogtinator commented 1 month ago

write_productcompose also returns a summary, but this is lost by solve_project. Not sure how this is meant to behave in case only product composer or both are used.

g7 commented 1 month ago

On a side-note: I wonder if moving

        if not self.skip_productcompose:
            self.write_productcompose()

from the bottom of solve_project() to the same place self.write_all_groups() is called would be okay just to keep things coherent.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 27.99%. Comparing base (ed30d28) to head (d297625).

Files Patch % Lines
pkglistgen/tool.py 0.00% 12 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3078 +/- ## ========================================== - Coverage 28.00% 27.99% -0.01% ========================================== Files 86 86 Lines 14967 14972 +5 ========================================== Hits 4191 4191 - Misses 10776 10781 +5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Vogtinator commented 1 month ago

On a side-note: I wonder if moving

        if not self.skip_productcompose:
            self.write_productcompose()

from the bottom of solve_project() to the same place self.write_all_groups() is called would be okay just to keep things coherent.

The core question is still

write_productcompose also returns a summary, but this is lost by solve_project. Not sure how this is meant to behave in case only product composer or both are used.

i.e. should it take one over the other (maybe validate equality?) or keep both?

g7 commented 1 month ago

@Vogtinator looks like write_productcompose() would always return an empty dictionary (perhaps a leftover?) so I think that's not used right now.

Vogtinator commented 1 month ago

@Vogtinator looks like write_productcompose() would always return an empty dictionary (perhaps a leftover?) so I think that's not used right now.

@adrianschroeter ^?

g7 commented 1 month ago

@Vogtinator are you ok if we merge this now in the meantime? It will unblock packagelist building

g7 commented 1 month ago

v2: after Fabian's suggestions, decoupled the summary dict creation from write_all_groups()/write_productcompose(). Please see the commit description for more details.