rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

Align estimated time savings between `Result` and `SourcesFileResults`

Open pstreef opened this issue 4 months ago • 3 comments

What's changed?

This PR solves 2 issues:

  1. Result only returns the estimated time savings from the first recipe that made a change to a file. This change sums up the estimated time of all changes to a file (however, this still does not take occurrences into account).

  2. SourcesFileResults has rows for all recipes in the hierarchy for tracability. Because each of these rows has an estimated time savings (of the actual change) summing the column gives the wrong values. By only setting the value to the leaf recipe this column becomes sum-able.

What's your motivation?

Aligning the estimated time savings and making the results easier to understand

Anything in particular you'd like reviewers to focus on?

There might have been a good reason to only expose the first value in Result.

Anyone you would like to review specifically?

@jkschneider

Have you considered any alternatives or workarounds?

We could chose to only go forward with either solution, however I expect this will remain hard to explain.

Note: that this solution works around the problem that we do not have a number of occurrences per Recipe (stack) in Result . If we would have that occurrence count (which would be a non trivial change) we could give an actual value and not just assume 1 change by each recipe that made a change.

Any additional context

Checklist

  • [x] I've added unit tests to cover both positive and negative cases
  • [x] I've read and applied the recipe conventions and best practices
  • [x] I've used the IntelliJ IDEA auto-formatter on affected files

pstreef avatar Sep 10 '25 09:09 pstreef

It sounds like somewhere along the line SourcesFileResults may have changed in a way that it wasn't intended originally. For a recipe stack A -> B -> C there should only be a single row with a recipe column of C and parent recipe column of B.

jkschneider avatar Sep 11 '25 09:09 jkschneider

It sounds like somewhere along the line SourcesFileResults may have changed in a way that it wasn't intended originally. For a recipe stack A -> B -> C there should only be a single row with a recipe column of C and parent recipe column of B.

Ok, so to solve the issue there we could remove the recursion altogether which adds the full stack. That solves 1 issue, but not the other. Do we want to keep the difference in the 2 summed estimated values?

pstreef avatar Sep 11 '25 11:09 pstreef

putting this in draft until @jkschneider re-reviews this

pstreef avatar Oct 02 '25 13:10 pstreef