Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dvc.lock updates #2139

Merged
merged 14 commits into from
Feb 3, 2021
Merged

dvc.lock updates #2139

merged 14 commits into from
Feb 3, 2021

Conversation

jorgeorpinel
Copy link
Contributor

Continuing #2131

@shcheklein shcheklein temporarily deployed to dvc-landing-jorge-mdwbtm76shyc February 2, 2021 05:14 Inactive
@jorgeorpinel jorgeorpinel marked this pull request as ready for review February 2, 2021 16:51
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-mdwbtm76shyc February 2, 2021 16:52 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-mdwbtm76shyc February 2, 2021 16:54 Inactive
@@ -364,7 +364,11 @@ validation and auto-completion.
> See also
> [How to Merge Conflicts](/doc/user-guide/how-to/merge-conflicts#dvcyaml).

### Output entries
### Simple output entries
Copy link
Member

Choose a reason for hiding this comment

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

hmm, I don't like this term "simple". There is nothing specifically simple or complex about those entries. It's totally fine to have them defined differently, with a bit different schema if we very clear in docs about this.

This comment was marked as resolved.

Copy link
Member

Choose a reason for hiding this comment

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

To my mind we are already in the section that describes the dvc.yaml schema and ideally it should be clear from the context that this is about dvc.yaml. I can see how additional message can help, but anything that comes to my mind would create even more confusion (people are not aware about other files, different types of outputs, etc).

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I simplified by calling the section in dvc.yaml "Output subfields" (which is what it is), PTAL.

If it's still confusing we could consider calling the sections in .dvc "Output/Dependency tracking entries".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p.s. TBH the ideal solution would be to have tables inside tables but that's not doable on md, would need some design and frontend work.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good!

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-mdwbtm76shyc February 3, 2021 03:09 Inactive
@jorgeorpinel
Copy link
Contributor Author

OK, merging then 🙂

@jorgeorpinel jorgeorpinel merged commit 60fb983 into master Feb 3, 2021
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.

2 participants