Skip to content

cmd: update push deps example#1591

Merged
jorgeorpinel merged 5 commits into
treeverse:masterfrom
imhardikj:pushcmd
Jul 29, 2020
Merged

cmd: update push deps example#1591
jorgeorpinel merged 5 commits into
treeverse:masterfrom
imhardikj:pushcmd

Conversation

@imhardikj
Copy link
Copy Markdown
Contributor

@imhardikj imhardikj commented Jul 18, 2020

Close #1445 by addressing the last pending checkbox:

  • push cmd ref.

@imhardikj imhardikj marked this pull request as draft July 18, 2020 22:15
@imhardikj imhardikj marked this pull request as ready for review July 18, 2020 22:37
Comment thread content/docs/command-reference/push.md Outdated
Comment thread content/docs/command-reference/push.md Outdated
Comment thread content/docs/command-reference/push.md Outdated
Comment thread content/docs/command-reference/push.md Outdated
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☝️

@imhardikj imhardikj changed the title Removing term Dvcfile in push cmdref push cmdref: removing term Dvcfile and updating example Jul 21, 2020
@imhardikj
Copy link
Copy Markdown
Contributor Author

@jorgeorpinel I have removed example-get started repo and instead set up a project for original example using dummy data and files.
I have used $cat dvc.yaml here currently but the dvc dag visualization of this dummy project can be viewed in this db08e42 commit.

@imhardikj imhardikj requested a review from jorgeorpinel July 21, 2020 21:14
@jorgeorpinel jorgeorpinel changed the title push cmdref: removing term Dvcfile and updating example cmd: update push example example Jul 22, 2020
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting there.

@imhardikj
Copy link
Copy Markdown
Contributor Author

@jorgeorpinel updated the example

@imhardikj imhardikj requested a review from jorgeorpinel July 22, 2020 14:09
@jorgeorpinel jorgeorpinel mentioned this pull request Jul 23, 2020
3 tasks
@jorgeorpinel jorgeorpinel changed the title cmd: update push example example cmd: update push deps example Jul 23, 2020
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but we need to wait until #1584 (comment) is merged before merging this one.

@shcheklein
Copy link
Copy Markdown
Contributor

hey, thanks @imhardikj , but I still see Dvcfile in the push.md - I think we should get rid of it completely to close this ticket?


```dvc
$ dvc push --with-deps matrix-train.p.dvc
$ dvc push --with-deps test-posts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to rewrite the example above, the project structure for this command to make sense?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, why do change the stage name here, intentional?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rewrite the example above, the project structure for this command to make sense

Yes! It was also done in the complementary PR. Both are merged, please check in https://dvc.org/doc/command-reference/push#example-with-dependencies

why do change the stage name here, intentional?

Intentional. I came up with these new names. But now that you mention it matrix-train would've been the obvious choice here 🤦

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Jul 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait no, that't the name of the next stage. That's why I came up with test-posts.

... Do some work based on the partial update

$ dvc push --with-deps model.p.dvc
$ dvc push --with-deps matrix-train
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here - why do we need to change the stage name?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think model is a good stage name.

@jorgeorpinel
Copy link
Copy Markdown
Contributor

thanks @imhardikj , but I still see Dvcfile in the push.md

Sorry that wsa confusing but I set it up that way: someone else was working on that part of the code so I asked Hardik to leave it alone. Now both PRs are merged and Dvcfile is gone from the code base 🙂

@imhardikj imhardikj deleted the pushcmd branch July 30, 2020 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

term: stop using Dvcfile

3 participants