Skip to content

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

Merged
efiop merged 9 commits into
masterfrom
protected-mode-by-default
Mar 17, 2020
Merged

docs: protected mode is now enabled by-default#1058
efiop merged 9 commits into
masterfrom
protected-mode-by-default

Conversation

@efiop
Copy link
Copy Markdown
Contributor

@efiop efiop commented Mar 16, 2020

treeverse/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
Comment thread public/static/docs/command-reference/config.md
Comment thread public/static/docs/user-guide/large-dataset-optimization.md Outdated
Comment thread public/static/docs/command-reference/config.md
@shcheklein shcheklein requested a review from jorgeorpinel March 16, 2020 19:41
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.

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

Comment thread public/static/docs/command-reference/config.md Outdated
Comment thread public/static/docs/command-reference/config.md Outdated
Comment thread public/static/docs/command-reference/unprotect.md
Comment thread public/static/docs/command-reference/unprotect.md Outdated
Comment thread public/static/docs/user-guide/large-dataset-optimization.md Outdated
Comment thread public/static/docs/user-guide/large-dataset-optimization.md Outdated
Comment thread public/static/docs/user-guide/large-dataset-optimization.md Outdated
Co-Authored-By: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 16, 2020 22:25 Inactive
Co-Authored-By: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 16, 2020 22:26 Inactive
Co-Authored-By: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@shcheklein shcheklein temporarily deployed to dvc-landing-protected-m-17pkzw March 16, 2020 22:26 Inactive
@efiop
Copy link
Copy Markdown
Contributor Author

efiop commented Mar 16, 2020

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

@jorgeorpinel
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

efiop commented Mar 17, 2020

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

Co-Authored-By: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@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
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 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
Copy Markdown
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
Copy Markdown
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 🙁

Comment thread public/static/docs/user-guide/large-dataset-optimization.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.

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
Copy Markdown
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
Copy Markdown
Contributor

@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