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: add Configuration guide #4379

Merged
merged 6 commits into from
Mar 16, 2023
Merged

guide: add Configuration guide #4379

merged 6 commits into from
Mar 16, 2023

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Mar 11, 2023

@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide labels Mar 11, 2023
@shcheklein shcheklein temporarily deployed to dvc-org-guide-config-dnq66kjws March 11, 2023 05:10 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-config-dnq66kjws March 11, 2023 05:14 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Mar 11, 2023

Link Check Report

There were no links to check!

Comment on lines 80 to 84
### core

- [`core.remote`](#remote) - name of the default remote storage

- `core.interactive` - whether to always ask for confirmation before reproducing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wonder whether these should be hidden in <details> sections, so the page doesn't seem that long.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried L2 headers instead of L3 so they all show in the sidebar?

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Mar 11, 2023

Choose a reason for hiding this comment

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

I guess that would also be better than now, yes. I'd still also consider using hidden sections... Maybe both? U: I tried combining both and it does work (when you click in the title on the right sidebar it scrolls and opens the section). Up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p.s. updated to use both for now, PTAL.

Comment on lines 11 to 13
`.dvc/config` is meant to be tracked by Git and should not contain sensitive
user info or secrets (passwords, SSH keys, etc). Other
[config file locations](#config-file-locations) can be used as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`.dvc/config` is meant to be tracked by Git and should not contain sensitive
user info or secrets (passwords, SSH keys, etc). Other
[config file locations](#config-file-locations) can be used as well.
`.dvc/config` is meant to be tracked by Git and should not contain sensitive
user info or secrets (passwords, SSH keys, etc). Other
[config file locations](#config-file-locations) are not tracked by Git and can
be used for sensitive info.

Minor suggestion to be more direct in this explanation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or use what's in the ref: Use --local when in doubt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, didn't love either suggestion. I updated it in a different way (b7e9672), PTAL.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's your goal with this admon? I was trying to make sure that we include enough info so people know where they generally should store sensitive info. I feel like just saying that other locations can be used kind of leaves people hanging.

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Mar 11, 2023

Choose a reason for hiding this comment

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

Good point. We could move it into the next section (Config file locations)... U: Done

Copy link
Contributor

Choose a reason for hiding this comment

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

@jorgeorpinel I'm not seeing an update here. Am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you pushed to #4380 instead of this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway, looks good there, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Glad it worked out in the end. Thanks

@shcheklein shcheklein temporarily deployed to dvc-org-guide-config-dnq66kjws March 11, 2023 19:56 Inactive
@jorgeorpinel jorgeorpinel self-assigned this Mar 11, 2023
@shcheklein shcheklein temporarily deployed to dvc-org-guide-config-dnq66kjws March 11, 2023 19:57 Inactive
@jorgeorpinel
Copy link
Contributor Author

My only other Q (other than #4379 (review) above) is whether we should find some more places to link to this guide, like https://dvc.org/doc/install. Either way this (and then #4380) should be mergeable I think 🙂

jorgeorpinel added a commit that referenced this pull request Mar 11, 2023
@jorgeorpinel jorgeorpinel mentioned this pull request Mar 13, 2023
1 task
* guide: Update links to config sections

Previously in the cmd ref, now in a new guide

* guide: some more links related to `dvc config` instances

* guide: update links from `dvc config` (ref) to new guide

* guide: clarify about other config file locations

per #4379 (review)

* guide: refinements

per #4379 (review)
and #4379 (comment)

* Update content/docs/user-guide/project-structure/configuration.md

---------

Co-authored-by: Dave Berenbaum <[email protected]>
@shcheklein shcheklein temporarily deployed to dvc-org-guide-config-dnq66kjws March 13, 2023 15:55 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-config-dnq66kjws March 13, 2023 16:09 Inactive
@dberenbaum dberenbaum merged commit 29f5ebd into main Mar 16, 2023
@dberenbaum dberenbaum deleted the guide/config branch March 16, 2023 16:16
@jorgeorpinel jorgeorpinel added the p1-important Active priorities to deal within next sprints label Mar 17, 2023
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 p1-important Active priorities to deal within next sprints
Projects
None yet
Development

Successfully merging this pull request may close these issues.

guide: extract part of theconfig cmd ref.
3 participants