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

cmd ref: remove unnecessary indices? #903

Merged
merged 11 commits into from
Jan 8, 2020
Merged

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Jan 7, 2020

Blocked by #891

Closes #727

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-engine-remo-rbc7o1 January 8, 2020 17:25 Inactive
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jan 8, 2020

So... #891 was only really for the contribution guide 😋 (from #903 (comment))

Actually, on this @shcheklein what about my #727 (comment) about cmd ref indices?

have useful, unique content but I'm not sure the cmd ref is the best place for those explanations. Should we take the opportunity to move them into a new user-guide: "Basic Concepts" (#550)?

Removing them would mean that the first subcommand opens by default though. For cache -> cache/dir (good), for metrics and remote -> */add, and for pipeline -> pipeline/list

@shcheklein
Copy link
Member

shcheklein commented Jan 8, 2020

@jorgeorpinel hmm ... I think they are valuable. Not matter what, but dvc remote -h is a legit command by itself and should be described. Or in the future we can decide to make dvc remote an alias of dvc remote list?

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jan 8, 2020

Yes, they have valuable content explaining what the concepts are (cache, metrics, pipeline, remote). My point is that such content probably doesn't belong in the command reference and we can move it to a new user-guide: "Basic Concepts" (per #550) – and once that is done, it won't make sense to keep these indices.

Should they just be a short list of links to their subcommands? Or remove completely and let the first one load. I think at least for dvc cache since it only has dir inside, we should just remove the index.

@shcheklein
Copy link
Member

@jorgeorpinel dvc remote, dvc cache, etc are valid commands. So I think they belong to the command reference and should have more or less the same structure.

@shcheklein

This comment has been minimized.

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jan 8, 2020

dvc remote, dvc cache, etc are valid commands

Ah yes good point. Didn't think about that. So we definitely need the indices. OK! This PR is now officially reviewable!

p.s. but please note that I've extracted the discussion about moving concepts explanations out of the cmd ref to #550 (comment).

@jorgeorpinel jorgeorpinel marked this pull request as ready for review January 8, 2020 19:34
Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

LGTM 👍 😎

@jorgeorpinel jorgeorpinel changed the title web: remove unnecessary cmd ref indices cmd ref: remove unnecessary indices? Jan 8, 2020
@jorgeorpinel jorgeorpinel merged commit 23c2de0 into master Jan 8, 2020
@shcheklein shcheklein deleted the engine/remove-indices branch January 22, 2020 18:13
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.

docs: remove unnecessary index pages that serve as a table of contents
2 participants