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

doc: add doc about foreach target #3989

Closed
wants to merge 1 commit into from
Closed

doc: add doc about foreach target #3989

wants to merge 1 commit into from

Conversation

skshetry
Copy link
Member

@skshetry skshetry commented Sep 23, 2022

Syncing with iterative/dvc#8352. I am skipping repro and exp run because they are well discussed in the Options sections, and we wanted to avoid describing too much and took the example-based approach in the docs.

See #1983. The support for foreach group name in the target was extended in iterative/dvc#8210.

@skshetry skshetry requested a review from dberenbaum September 23, 2022 08:05
@skshetry skshetry self-assigned this Sep 23, 2022
@shcheklein shcheklein temporarily deployed to dvc-org-doc-foreach-hdx3pheske September 23, 2022 08:05 Inactive
@github-actions
Copy link
Contributor

5662ee8

Link Check Report

There were no links to check!

CML watermark

@@ -12,7 +12,7 @@ usage: dvc checkout [-h] [-q | -v] [--summary] [-d] [-R] [-f]

positional arguments:
targets Limit command scope to these tracked files/directories,
.dvc files, or stage names.
.dvc files and stage or foreach-group names.
Copy link
Contributor

@daavoo daavoo Sep 23, 2022

Choose a reason for hiding this comment

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

I am not sure if people would know what foreach-group means.

It is not used in https://dvc.org/doc/user-guide/project-structure/dvcyaml-files#foreach-stages which are the only existing docs for foreach

Copy link
Contributor

@jorgeorpinel jorgeorpinel Sep 28, 2022

Choose a reason for hiding this comment

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

Agreed. I don't think this change is needed TBH: foreach groups ultimately become stages so it's covered with "or stage names". No need to be that super explicit about every possible syntax in the help output, especially for such an advanced feature (not that used from what I understand).

  • Explaining that you can target them in the foreach docs linked above should be enough IMO.

@dberenbaum
Copy link
Contributor

Thanks, @skshetry!

I am skipping repro and exp run because they are well discussed in the Options sections, and we wanted to avoid describing too much and took the example-based approach in the docs.

Should we add a foreach example in the options section then? I don't see foreach mentioned anywhere. Also, same with stage list.

Comment on lines +12 to +13
targets Limit command scope to these tracked files/directories,
.dvc files and stage or foreach-group names.
Copy link
Contributor

@jorgeorpinel jorgeorpinel Sep 28, 2022

Choose a reason for hiding this comment

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

Also, the syntax here is unclear. Probably revert to a comma if we keep this? Maybe

Suggested change
targets Limit command scope to these tracked files/directories,
.dvc files and stage or foreach-group names.
targets Limit command scope to these tracked files/directories,
.dvc files, stage (or foreach) names.

Rel. iterative/dvc#8352 (review)

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Sep 28, 2022

add a foreach example in the options section then?

Adding that to targets (under https://dvc.org/doc/command-reference/repro#options) as well as possibly a stand-alone example could help indeed since this topic is only covered in the dvc.yaml ref atm and we want cmd refs to be as comprehensive/ self-contained as possible.

p.s. note that the targets arg in the Options section doesn't have a 🔗 anchor like all the options, probably because it doesn't correspond to an actual flag/option arg -- if that's a problem we can check what to do about it with the websites team. Cheers

@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) C: ref Content of /doc/*-reference C: guide Content of /doc/user-guide labels Sep 28, 2022
@dberenbaum
Copy link
Contributor

Closed in favor of #4010

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 C: ref Content of /doc/*-reference
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants