Skip to content

ref: add config --project flag and standardize the others#2064

Merged
jorgeorpinel merged 35 commits into
masterfrom
efiop-dvc-5179
Mar 31, 2021
Merged

ref: add config --project flag and standardize the others#2064
jorgeorpinel merged 35 commits into
masterfrom
efiop-dvc-5179

Conversation

@efiop
Copy link
Copy Markdown
Contributor

@efiop efiop commented Dec 30, 2020

Per treeverse/dvc#5179 (and treeverse/dvc#5399)

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.

Please choose to allow us to edit your branch when creating the PR.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

@shcheklein shcheklein temporarily deployed to dvc-landing-efiop-dvc-5-r7iq4r December 30, 2020 01:31 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-efiop-dvc-5-r7iq4r December 30, 2020 01:40 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-efiop-dvc-5-r7iq4r December 30, 2020 01:45 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-efiop-dvc-5-r7iq4r December 30, 2020 17:58 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-efiop-dvc-5-r7iq4r December 30, 2020 18:00 Inactive
@efiop efiop requested a review from jorgeorpinel December 30, 2020 18:01
@efiop efiop changed the title [WIP] docs: add --repo flag for config/remote docs: add --repo flag for config/remote Dec 30, 2020
@efiop efiop changed the title docs: add --repo flag for config/remote [WIP] docs: add --repo flag for config/remote Dec 30, 2020
@shcheklein shcheklein temporarily deployed to dvc-landing-efiop-dvc-5-r7iq4r December 30, 2020 18:08 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-efiop-dvc-5-r7iq4r December 30, 2020 18:09 Inactive
@efiop efiop changed the title [WIP] docs: add --repo flag for config/remote docs: add --repo flag for config/remote Dec 30, 2020
Comment thread content/docs/command-reference/cache/dir.md Outdated
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

A first quick suggested text for cache dir which can probably be generalized in other docs.

But I'm still not sure about the term repo vs. project... ⌛

Comment thread content/docs/command-reference/cache/dir.md Outdated
Comment thread content/docs/command-reference/cache/dir.md Outdated
@jorgeorpinel jorgeorpinel mentioned this pull request Jan 5, 2021
12 tasks
Comment thread content/docs/command-reference/cache/dir.md Outdated
Comment thread content/docs/command-reference/cache/dir.md Outdated
Comment thread content/docs/command-reference/config.md Outdated
@jorgeorpinel
Copy link
Copy Markdown
Contributor

@efiop this change is bigger in docs than it seems from the OP and I only started reviewing for now. We may need several iterations. Thanks a lot for contributing this as we are pretty busy with the other 2.0 docs updates at the moment (see #2026).

efiop pushed a commit to efiop/dvc that referenced this pull request Feb 3, 2021
Per treeverse/dvc.org#2064

Not changing the internal terminology because we stick to Repo in a lot
of places there and adjusting all of it would be wasteful. Maybe in the
future.
@efiop efiop self-assigned this Feb 23, 2021
jorgeorpinel and others added 9 commits March 1, 2021 05:55
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@efiop
Copy link
Copy Markdown
Contributor Author

efiop commented Mar 18, 2021

@jorgeorpinel Could you take a look when you have time, please?

@jorgeorpinel
Copy link
Copy Markdown
Contributor

@efiop sorry, this is one of the few ones that fell through the cracks but I'll take it over now. Thanks 👍

@jorgeorpinel jorgeorpinel assigned jorgeorpinel and unassigned efiop Mar 28, 2021
Comment on lines +64 to +66
- `--project` - save remote configuration to the project's config
(`.dvc/config`). Used by default.

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Mar 28, 2021

Choose a reason for hiding this comment

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

Q @efiop: if this is used by default and remote add only writes to config, why even have the option here? When does it have an effect?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same for remote modify/remove/rename

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For symmetry mostly. Right now remtoe add/modify/remove/rename will just default to --project anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @efiop. So just to be clear, the option in these commands never has an effect right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is the default behavior.

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Just a Q above but this is mergeable I think.

@jorgeorpinel jorgeorpinel changed the title docs: add --project flag for config/remote ref: add config --project flag and standardize the others Mar 28, 2021
@jorgeorpinel jorgeorpinel removed their assignment Mar 28, 2021
@jorgeorpinel jorgeorpinel merged commit e18739e into master Mar 31, 2021
@jorgeorpinel
Copy link
Copy Markdown
Contributor

Merged but we may want to delete the --project option from remote add/modify/remove/rename if it has no effect (see #2064 (review)).

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.

6 participants