implementing forge trace - #397
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #397 +/- ##
==========================================
+ Coverage 52.28% 52.32% +0.03%
==========================================
Files 37 37
Lines 6049 6064 +15
Branches 648 648
==========================================
+ Hits 3163 3173 +10
- Misses 2238 2243 +5
Partials 648 648 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I thought some more about it and decided to rename the new column to |
nevrome
left a comment
There was a problem hiding this comment.
Thanks for looking into this! @TCLamnidis will be happy to hear that this will be added soon. I just have three comments:
- I think adding/updating this new column should not be the default, but a
forgeoption. We could call it--traceor--docSourcePacor something like this. To illustrate why I would find this annoying as a default behaviour: I recently prepared the patched versions of the AADR v66 packages. There I wanted a minimal diff to keep things human-readable. A new column would have been very irritating in this context. SourcePackagedoes not fit with the usual column naming scheme in Pascal_Snake_Case. Maybe we should go withSource_Package.- Maybe we should add a more complex test case where a package with the
Source_Packagecolumn gets forged a second time, to see the concatenation in action. Maybe even from two different source packages.
|
Yes to all! One advantage of fixing 1) is that the golden tests won't all get updated. I'll get to it. |
Renamed the new Janno column to "Source_Package"
|
Done. Can you check again @nevrome ? Thanks |
|
Great! I tested it and found it working 👍. There is a detail I think could be improved: When you merge a package A that already has a Beyond that I found the command line documentation a bit unclear. We also don't use Finally, the feature should also be added to the release changelog text. When this is merged we can make a new trident release. |
|
OK, agreed to all three points, will get them done. Thanks. |
This PR implements a Forge Trace, which addresses #251.
The implementation is radically simple:
ForgeTracealready exists, a new trace is appended with;.What do you think?
The package is not hard to review, the only relevant changes are in Forge.hs. But of course there are tons of Forge Test packages, which all had to be updated for the golden tests to pass.