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

guide: updates to dvc.yaml spec #3789

Merged
merged 88 commits into from
Oct 12, 2022
Merged

guide: updates to dvc.yaml spec #3789

merged 88 commits into from
Oct 12, 2022

Conversation

jorgeorpinel
Copy link
Contributor

Extracted from #3414 (comment)

This page should focus on the schema/spec of the file and it's sections more than guidance and explaining concepts, since we'll have a guide for that now (see #3414).

@shcheklein shcheklein temporarily deployed to dvc-org-guide-dvc-yaml-rx4eyee August 2, 2022 08:53 Inactive
@jorgeorpinel jorgeorpinel force-pushed the iesahin/ug-pipelines branch 2 times, most recently from 5614200 to ef24051 Compare August 31, 2022 01:04
Base automatically changed from iesahin/ug-pipelines to main September 7, 2022 22:25
@jorgeorpinel jorgeorpinel self-assigned this Sep 29, 2022
@jorgeorpinel jorgeorpinel added the p2-nice-to-have Less of a priority at the moment. We don't usually deal with this immediately. label Sep 29, 2022
@dberenbaum
Copy link
Contributor

@jorgeorpinel What's the status of this PR? Is it still needed?

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Oct 5, 2022

I'm not sure how this PR got so complicated 😓 but yes, after mergingmain (resolved a bunch of conflicts) I see many useful changes here. I will have to split this though, otherwise it's too hard to review... ⏳

@jorgeorpinel jorgeorpinel changed the title guide: updates to and around dvc.yaml doc guide: updates to dvc.yaml ref Oct 7, 2022
@jorgeorpinel jorgeorpinel changed the title guide: updates to dvc.yaml ref guide: updates to dvc.yaml spec Oct 7, 2022
Comment on lines +205 to +209
## Stage entries

These are the fields that are accepted in each stage:

| Field | Description |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All this I just moved up unchanged. It started in old line 506 before.

Copy link
Contributor

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

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

Thanks @jorgeorpinel! LGTM except confused why it says 88 commits?

It would help me better manage the docs workload if we can be a little more careful about what changes are worthwhile. If the PR was all about making it more of a spec and less of a guide it would make more sense, but most changes look like minor, subjective style issues. If we can reduce some of those, I hope it can help reduce noise and focus more on what's important. I know you don't intend for me to have to review everything right away, but it would help to cut out the unimportant stuff altogether instead of having to try to figure it out as I review the PR. 🙏

What do you think?

@jorgeorpinel
Copy link
Contributor Author

It's 88 commits because this was started in July. I later removed a bunch of changes.

Agree about the relevance of these changes. At that time I already had a bunch of edits stashed for this file which I didn't manage to propose since they never seemed major enough so eventually I just made the PR (and then it grew out of control).

But don't worry, in the new docs process, this kind of work is not something I'll be paying much attention to in the foreseeable future.

@jorgeorpinel jorgeorpinel merged commit 548749a into main Oct 12, 2022
@jorgeorpinel jorgeorpinel deleted the guide/dvc.yaml branch October 12, 2022 03:35
@github-actions
Copy link
Contributor

548749a

Link Check Report

There were no links to check!

CML watermark

@jorgeorpinel jorgeorpinel added type: enhancement Something is not clear, small updates, improvement suggestions and removed ⌛ status: wait-core-merge Waiting for related product PR merge/release p2-nice-to-have Less of a priority at the moment. We don't usually deal with this immediately. labels Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide type: enhancement Something is not clear, small updates, improvement suggestions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants