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

docs: protected mode is now enabled by-default #1058

Merged
merged 9 commits into from
Mar 17, 2020
Merged

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Mar 16, 2020

iterative/dvc#3472

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 chose 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-protected-m-17pkzw March 16, 2020 18:15 Inactive
@shcheklein shcheklein requested a review from jorgeorpinel March 16, 2020 19:41
Copy link
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.

Good stuff. Some comments below. We can take it over at some point if needed, up to you guys 🙂

public/static/docs/command-reference/config.md Outdated Show resolved Hide resolved
public/static/docs/command-reference/config.md Outdated Show resolved Hide resolved
public/static/docs/command-reference/unprotect.md Outdated Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 16, 2020 22:25 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 16, 2020 22:26 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 16, 2020 22:26 Inactive
@efiop
Copy link
Contributor Author

efiop commented Mar 16, 2020

Will take a fresh look in the morning. Thanks for the reviews! @jorgeorpinel 🙏

@jorgeorpinel
Copy link
Contributor

Thank you! Only my comments on public/static/docs/user-guide/large-dataset-optimization.md are pending now, it seems.

@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 17, 2020 16:32 Inactive
@efiop efiop force-pushed the protected-mode-by-default branch from ec32d04 to 14ecc26 Compare March 17, 2020 16:38
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 17, 2020 16:38 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 17, 2020 16:39 Inactive
@efiop efiop force-pushed the protected-mode-by-default branch from 53453ad to 89fa3e4 Compare March 17, 2020 16:40
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 17, 2020 16:40 Inactive
@efiop
Copy link
Contributor Author

efiop commented Mar 17, 2020

@jorgeorpinel Addressed all of your comments, please take a look when you'll have time. Thanks!

@efiop efiop force-pushed the protected-mode-by-default branch from 89fa3e4 to 73447dc Compare March 17, 2020 16:55
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 17, 2020 16:56 Inactive
Copy link
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 couple more details. Let me try to commit the suggestions myself here on GH 😬

Comment on lines 42 to 43
protect against that, DVC makes hard/soft links read-only, forcing the user to
use `dvc unprotect` before modifying them. Finally, a 4th "linking" alternative
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
protect against that, DVC makes hard/soft links read-only, forcing the user to
use `dvc unprotect` before modifying them. Finally, a 4th "linking" alternative
protect against that, DVC makes hardlinks and symlinks links read-only, which requires the user to
use `dvc unprotect` before modifying them.
Finally, a 4th "linking" alternative

Needs rewrap... Let's see if Restyled can fix this ⏳

Copy link
Contributor

Choose a reason for hiding this comment

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

Strange. No Restyled difference 🙁

Copy link
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.

Done Ruslan! But there's a formatting check failing and Restyled isn't fixing it? See #1058 (comment)

@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 17, 2020 17:09 Inactive
@efiop
Copy link
Contributor Author

efiop commented Mar 17, 2020

@jorgeorpinel Maybe it is not configured right, I haven't looked deeply in our dvc.org setup. Fixed manually.

@efiop efiop merged commit 25f9652 into master Mar 17, 2020
@efiop efiop deleted the protected-mode-by-default branch March 17, 2020 17:12
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.

Cool stuff, thanks @efiop and @jorgeorpinel !

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.

3 participants