cmd: status examples #1952
cmd: status examples #1952
Conversation
|
Is this still a draft PR though? |
|
@jorgeorpinel One example of |
There was a problem hiding this comment.
One example of fetch that requires indentation correction is having a merge conflict
I don't see the merge conflict anymore and I see this is a regular PR so I'm approving.
BUT there's one or two minor details left, mentioned above, so not merging yet. Pleas ping me when ready @imhardikj. Thanks
| dofoo: | ||
| changed deps: | ||
| modified: bar | ||
| changed outs: | ||
| not in cache: foo | ||
| dobar: | ||
| changed outs: | ||
| deleted: bar |
There was a problem hiding this comment.
But now we need to worry about the consistency of the example. How come dofoo comes first but it uses bar as a dependency? bar is the output of dobar which comes after...
Please try to actually codify these examples in a dvc.yaml file and run repro/status as needed so they make sense.
There was a problem hiding this comment.
changing deps of dofoo to baz make it is consistent. It will become a simple pipeline:
+---------+
| baz.dvc |
+---------+
*
*
*
+-------+
| dofoo |
+-------+
*
*
*
+-------+
| dobar |
+-------+
Considering that foo is a dependency of dobar.
There was a problem hiding this comment.
Better! But the current example doesn't show a connection between dofoo and dobar. In fact dobar is a separate pipeline altogether as it has no dependencies.
There was a problem hiding this comment.
Have you actually codified it in dvc.yaml so we can see what the output would be? I'm not sure about how status orders stages, maybe it's chronological, maybe it's alphabetical?
There was a problem hiding this comment.
Yes I actually made this pipeline. The output of status was correct as it was after executing dofoo stage with --no-commit. In the new update it shows connection between stages.
There was a problem hiding this comment.
So it prints baz.dvc at the end
Most of times baz.dvc was in starting. If you check in commit f0ff0ee, I moved baz.dvc to starting. Here in this review #1952 (comment), output of status was correct was actually meant for baz.dvc at starting.
But in the next commit b54bf7b I moved it back to last because I was testing this and got this output once, so I thought this was also correct. But yes majority of times baz.dvc was in starting only that's why I changed it in f0ff0ee.
There was a problem hiding this comment.
It should be in the beginning. I'm glad I double checked this... I'll fix it later.
There was a problem hiding this comment.
Can I open the PR for this. There's one more example remaining in fetch which requires indentation change. I removed it from this PR as it was showing merge conflict. #1952 (comment)
There was a problem hiding this comment.
I have the change stashed locally already, so it will get to the repo eventually. But feel free to send a PR, sure. No big deal either way.
There was a problem hiding this comment.
p.s. don't shy away from merge conflicts 🙂
|
Done 🙂 |
Partially Addresses #1824: