iterative
`--append` to reports
As discussed here is very helpful for the viewer to keep backwards compatibility to keep the previous comment collapsed.
<details id="SHA">
<summary>SHA</summary>
CML REPORT
</details>
I'd say if the platform doesn't support edit history, then users requiring history should simply not use --update (just use --pr). I'd downgrade this to p2 or even just close?
I'd downgrade this to p2 or even just close?
Please follow up the viewer link. They find this feature very helpful
Are you talking about https://github.com/iterative/viewer/issues/2759? I already commented there and don't see any connection. @duijf do let me know if https://github.com/iterative/cml/issues/803#issuecomment-963700819 is not enough for Studio.
They need the collapsed history to be able to not loose the commit report in Viewer @casperdcl
In Studio we currently link CML reports to specific commits like this:
If a new commit is pushed, we add a new row to this table. If we have a CML report for this commit, we would display another GH icon to link to that report/comment.
PR comments challenge this assumption: since the CML PR comments can be updated in place, this association from a commit -> CMl report can become stale over time. We can address this in two ways:
- Remove stale links if a CML PR report is updated.
- Have CML keep the previous version of the report around and link to that historical report. (GH does not have a way to link to comment revisions)
IMO, option 2) would make for better UX
@casperdcl suggested in https://github.com/iterative/viewer/issues/2759#issuecomment-963698767 that we can implement something and push the burden on the user for configuring a CML template to integrate with this linking feature, but I'm not sure if users are currently able to include the previous version of the report in a template?
It's debatable how important this is. I'd like to get started with supporting CML PR comments in Studio soon and will probably implement 1) first if this feature does not exist in CML yet.
Hope that clarifies things! :)
@duijf it seems that you are effectively talking about asking users to not use the --update flag. Not using the flag would preserve all reports (default behaviour).
Not sure if we should be making CML itself preserve reports in-place with collapsible headings if --update is used. I'd suggest a new (feature request) flag --append for that...
That sounds like a fair description. Whether you want to expose this suggested behavior using a separate flag or make it the default for --update is up to you of course :)
