remote: add description for verify option #937
Conversation
|
@pared also, is there a PR in the core repo that implements this? Would be nice to put that in the description of this PR. (The template text you can leave or just remove after reading it.) Another hint: in the future in order to make docs process a little faster please use a branch in this repo directly, as opposed to your fork. this way it deploys an automatic review app (for sanity checks) and its always possible/easier for others to checkout/commit to small refinements (like the copy edits above). Thanks |
|
@jorgeorpinel please don't hesitate to apply small changes right away and let's just confirm with @pared the latest version that is ready to merge + if we have any questions to clarify (better ask in the engineering team or Discord then) |
|
I don't think I can on pared's fork/branch:
Maybe the "allow maintainers to commit..." option wasn't selected when opening the PR. |
There was a problem hiding this comment.
OK I was able to manually add the fork as Git remote, checkout the branch, and push to it so I addressed everything. (Not sure why GH won't let me commit the suggestions directly) @pared or @shcheklein please approve/merge 🙂
|
@pared @jorgeorpinel looks good to me, but I think it worth mentioning what remote types have this setting on/off by default. @jorgeorpinel might worth mentioning that we take this checksums from the DVC-files. |
This I don't know. I asked in the #engineering channel and Ruslan mentioned only on Gdrive. Added corresponding notes in 1185efc. He also mentioned its still a WIP PR on the core repo. So we shouldn't merge this just yet. (Another reason to try including the link to the core PR on these docs PRs.) |
There was a problem hiding this comment.
@pared please let us know when the original PR is merged in the core repo, thanks.
Done as well @shcheklein |
|
Ok, thanks @jorgeorpinel for taking time to configure your repo, Ill remember to push to origin branch, didn't know that sanity checks are triggered only on branch PR's. |
|
Original change has been merger @shcheklein @jorgeorpinel |
I believe it runs CI checks on a fork as well, but does not deploy on Heroku automatically to review |
|
@jorgeorpinel merging this? |
Ah yeah the CI checks run. I meant that a review app is deployed on Heroku so we can do manual sanity checks visually on a rendered website. But yeah thanks and np @pared
OK, merging here as well 😬 |
|
@jorgeorpinel looks like build fails after this merge |
Disregard the recommendations below if you use Edit on GitHub button to improve the docs 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 enables GitHub to link the PR to the corresponding bug and close it automatically when PR is merged.Thank you for the contribution - we'll try to review and merge it as soon as possible. 🙏
related to iterative/dvc#3200