ref: rewrap a couple usage blocks by jorgeorpinel · Pull Request #3345 · treeverse/dvc.org

jorgeorpinel · 2022-03-09T08:14:24Z

Per #3325 (review) and #3325 (review) originally

Some quality-related ideas on what criteria to use when formatting usage blocks. Potentially we can later match this in the core repo too (-h help output).

Establish clear-enough criteria - see ref: rewrap a couple usage blocks #3345 (review)
Update core repo as well - done in run/repro/stage add: regroup options/flags dvc#7524
Update all other commands both in core and docs - dvc: regroup all command options/flags [quality] dvc#7525

per #3325 (review) and #3325 (review)

daavoo

Is there a way to handle this criteria in the auto-formatter/linter?

jorgeorpinel · 2022-03-15T17:23:55Z

Meaning the line length specific to code blocks? I don't think so unfortunately, in fact md code blocks are allowed any line width by the linter, I think. We should always sanity check the rendered review app before merging -- that's how I detect code blocks with scroll bars.

dberenbaum · 2022-03-15T19:54:06Z

-usage: dvc repro [-h] [-q | -v] [-f] [-s] [-m] [--dry] [-i]
-                 [-p] [-P] [-R] [--no-run-cache] [--force-downstream]
-                 [--no-commit] [--downstream] [--pull] [--glob]
+usage: dvc repro [-h] [-q | -v] [-f] [-s] [-m] [-i] [-p] [-P] [-R]


So this groups all the short flags at the start? Looks good to me.

But then there's additional reordering within some of the long flags. Can you give some rationale for the order @jorgeorpinel?

I'm not sure if there was any rationale for the current order. cc @iterative/dvc If you have any context for the order of arguments under these commands, please add your thoughts 🙏 .

Yeah I think that on the core repo, currently new arguments are just added at the bottom so there's no logical order.

My criteria here isn't super defined but broadly: put general ones in the first line, esp. if they're shot flags; Then group by functionality in the order in which we think people will use them most -- in the case above (repro) realted to deps/params, then outs/metrics/plots, then DVCLive.

If we move [--no-exec] [--no-run-cache] [--no-commit] to the bottom for run and stage add, can we put the related repro options [--pull] [--no-run-cache] [--no-commit] at the end here?

Should the ordering criteria be translated back to dvc core repo?

Yeah, once there is an agreement on the order of everything, we should update the core repo.

move [--no-exec] [--no-run-cache] [--no-commit] to the bottom for run

Done. Well, I moved them above the DVCLive ones. PTAL

and stage add

Doesn't have them.

put the related repro options [--pull] [--no-run-cache] [--no-commit] at the end

Done.

BTW run --no-exec is equivalent to repro --dry right? May want to rename the former to "dry" as well.

BTW run --no-exec is equivalent to repro --dry right? May want to rename the former to "dry" as well.

I think --dry is reserved for commands that are read-only and make no actual changes to the repo state, whereas run --no-exec is basically dvc stage add and still writes the stage to dvc.yaml.

repro is not read-only; It changes the repo state. So should its --dry be renamed --no-exec then? UPDATE: Moved to treeverse/dvc#7522

dberenbaum · 2022-03-15T20:13:59Z

+               [--plots <path>] [--plots-no-cache <path>]
+               [--live <path>] [--live-no-cache <path>]
+               [--live-no-html] [-w <path>]
               [--always-changed] [--external] [--desc <text>]


And these are stage-level options?

Least used options

per #3345 (comment)

iesahin · 2022-03-30T14:29:13Z

I approved this again but is this an epic?

jorgeorpinel · 2022-03-30T21:38:48Z

is this an epic?

No but there are follow up tasks (I'll make an issue after this or something).

Co-authored-by: Dave Berenbaum <dave@iterative.ai>

…to dvclive/refs-rewrap

per #3345 (review)

* ref: rewrap a couple usage blocks per #3325 (review) and #3325 (review) * ref: reorg repro/run/stage add usage block * move run/repro exec modifyer options down per #3345 (comment) * Update content/docs/command-reference/repro.md Co-authored-by: Dave Berenbaum <dave@iterative.ai> * ref: make run options like stage add per #3345 (review) Co-authored-by: Dave Berenbaum <dave@iterative.ai>

ref: rewrap a couple usage blocks

c5ca3da

per #3325 (review) and #3325 (review)

jorgeorpinel requested a review from daavoo March 9, 2022 08:14

shcheklein temporarily deployed to dvc-org-dvclive-refs-re-fgvayc March 9, 2022 08:14 Inactive

jorgeorpinel marked this pull request as ready for review March 9, 2022 08:26

jorgeorpinel marked this pull request as draft March 9, 2022 08:26

jorgeorpinel commented Mar 9, 2022

View reviewed changes

Comment thread content/docs/command-reference/run.md Outdated

jorgeorpinel temporarily deployed to dvc-org-dvclive-refs-re-fgvayc March 9, 2022 08:44 Inactive

jorgeorpinel commented Mar 9, 2022

View reviewed changes

Comment thread content/docs/command-reference/run.md Outdated

jorgeorpinel requested a review from shcheklein March 9, 2022 08:47

jorgeorpinel added the type: discussion Requires active participation to reach a conclusion. label Mar 9, 2022

jorgeorpinel requested a review from iesahin March 9, 2022 08:48

ref: reorg repro/run/stage add usage block

0491087

jorgeorpinel force-pushed the dvclive/refs-rewrap branch from d131954 to 0491087 Compare March 9, 2022 08:51

jorgeorpinel temporarily deployed to dvc-org-dvclive-refs-re-fgvayc March 9, 2022 08:51 Inactive

jorgeorpinel self-assigned this Mar 10, 2022

iesahin approved these changes Mar 14, 2022

View reviewed changes

daavoo approved these changes Mar 15, 2022

View reviewed changes

jorgeorpinel marked this pull request as ready for review March 15, 2022 17:27

jorgeorpinel requested review from dberenbaum and removed request for shcheklein March 15, 2022 17:27

dberenbaum reviewed Mar 15, 2022

View reviewed changes

Comment thread content/docs/command-reference/run.md

dberenbaum reviewed Mar 15, 2022

View reviewed changes

Comment thread content/docs/command-reference/run.md Outdated

weekly-digest Bot mentioned this pull request Mar 23, 2022

Weekly Digest (23 February, 2020 - 1 March, 2020) treeverse/dvc#3425

Closed

jorgeorpinel added 2 commits March 28, 2022 19:47

Merge branch 'master' into dvclive/refs-rewrap

0883741

move run/repro exec modifyer options down

630fb6a

per #3345 (comment)

jorgeorpinel deployed to dvc-org-dvclive-refs-re-fgvayc March 29, 2022 02:05 View deployment

jorgeorpinel requested a review from dberenbaum March 29, 2022 02:08

iesahin approved these changes Mar 30, 2022

View reviewed changes

dberenbaum reviewed Mar 30, 2022

View reviewed changes

Comment thread content/docs/command-reference/repro.md Outdated

dberenbaum reviewed Mar 30, 2022

View reviewed changes

Comment thread content/docs/command-reference/run.md Outdated

jorgeorpinel and others added 3 commits March 30, 2022 15:41

Merge branch 'master' into dvclive/refs-rewrap

1300083

Update content/docs/command-reference/repro.md

d3c7aae

Co-authored-by: Dave Berenbaum <dave@iterative.ai>

Merge branch 'dvclive/refs-rewrap' of github.com:iterative/dvc.org in…

3327af1

…to dvclive/refs-rewrap

jorgeorpinel requested a deployment to dvc-org-dvclive-refs-re-fgvayc March 30, 2022 21:43 Abandoned

jorgeorpinel requested a deployment to dvc-org-dvclive-refs-re-fgvayc March 30, 2022 21:49 Abandoned

ref: make run options like stage add

cb88988

per #3345 (review)

jorgeorpinel temporarily deployed to dvc-org-dvclive-refs-re-fgvayc March 30, 2022 21:52 Inactive

jorgeorpinel merged commit a997ec2 into master Mar 30, 2022

jorgeorpinel deleted the dvclive/refs-rewrap branch March 30, 2022 21:53

This was referenced Mar 31, 2022

repro: rename --dry to --no-exec treeverse/dvc#7522

Closed

run/repro/stage add: regroup options/flags treeverse/dvc#7524

Merged

dvc: regroup all command options/flags [quality] treeverse/dvc#7525

Closed

jorgeorpinel mentioned this pull request Sep 6, 2022

ref: data status #3812

Merged

jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) C: ref Content of /doc/*-reference p2-nice-to-have Less of a priority at the moment. We don't usually deal with this immediately. labels Mar 16, 2023

Conversation

jorgeorpinel commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daavoo left a comment

Choose a reason for hiding this comment

Uh oh!

jorgeorpinel commented Mar 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daavoo Mar 25, 2022 • edited by jorgeorpinel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgeorpinel Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgeorpinel Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iesahin commented Mar 30, 2022

Uh oh!

Uh oh!

Uh oh!

jorgeorpinel commented Mar 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jorgeorpinel commented Mar 9, 2022 •

edited

Loading

jorgeorpinel commented Mar 15, 2022 •

edited

Loading

daavoo Mar 25, 2022 •

edited by jorgeorpinel

Loading

jorgeorpinel Mar 29, 2022 •

edited

Loading

jorgeorpinel Mar 31, 2022 •

edited

Loading