plots and metrics: update new targets rules#1809
Conversation
jorgeorpinel
left a comment
There was a problem hiding this comment.
Done here! Thanks @pared
|
Please merge when you deem appropriate @pared (and feel free to include treeverse/dvc#4620 (review) here). |
|
@pared looks good to me! let's resolve conflicts and ship it :) |
|
From #1809 (review):
And from #1809 (review):
I don't think we've done this yet though @shcheklein @pared. That would imply a change in the plots cmd ref index, no? |
8c69691 to
f4e6333
Compare
|
@jorgeorpinel I agree that we need to update the description of the commands, but we need to finish up here first. |
True. But I thought the idea was that we wouldn't need to mention it in those or at least not stress it much as long as it was clearly expressed in the index that DVC can work with any valid plots file whether or not in dvc.yaml. |
There was a problem hiding this comment.
That said I agree we can merge this with the notes so far, although the index still needs a small update so I unlinked this from #1800
Also the note in plots show could probably be in a different place: not sure why it's part of the first paragraph, seems disconnected with the other sentences there (see #1809 (review))
jorgeorpinel
left a comment
There was a problem hiding this comment.
Wait... What about https://dvc.org/doc/command-reference/plots/modify?
|
@jorgeorpinel well, in this case, plot need to be defined in |
Ah, makes sense @pared . I'm not suggesting that we support that BTW, I just didn't think it through. Thanks p.s. I see you created treeverse/dvc/issues/4741 about the error message 👍 |

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.
🐛 Please make sure to mention
Fix #issue(if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.Please choose to allow us to edit your branch when creating the PR.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏
Addresses #1800
Related to treeverse/dvc#4590