cmd ref: import - support for non-DVC repos #1339
Conversation
| ```dvc | ||
| $ dvc import git@github.com:GSA/data \ | ||
| enterprise-architecture/it-standards.csv | ||
| Importing 'enterprise-architecture/it-standards.csv |
There was a problem hiding this comment.
minor: is the output actually split? if not - let's keep it as-is, for output we tend to preserve it as we have it in DVC
There was a problem hiding this comment.
Okay, but that makes the code div scrollable. Is that okay?
There was a problem hiding this comment.
yes, it's fine ... it's better to keep it close to the actual output
There was a problem hiding this comment.
Hm I would personally prefer non-scrollable code/cli samples. No right or wrong answer here but for consistency should we go for non-scrolling? Most code/CLI samples try to avoid the horizontal scrolling AFAIR (that means up to 72 character-long lines).
There was a problem hiding this comment.
there are two types of lines:
- commands - start with
$. It's reasonable to avoid the horizontal scroll. There is a mechanism for that -\. - output - this is one is not that simple. If we start splitting output w/o any special sign that we wrapped it there is a danger at some point that we can get a different semantic for it. If it looks like it's too much I would try simplify with
...instead, or something like that. But not silently introduce newlines that are actually not there.
There was a problem hiding this comment.
Good points, I use ... often to shorten these output lines. No need to keep the whole thing, just give an idea of what to expect from running commands and only when absolutely needed (best to avoid sample command output altogether). Please note, @utkarshsingh99
shcheklein
left a comment
shcheklein
left a comment
There was a problem hiding this comment.
looks cool 😎 , just one minor comment
Review changes - I
@utkarshsingh99 please link the PR properly to the original issue so it's closed automatically when this PR merges. See https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue For future reference, I would also suggest to link directly to the specific comment this addresses in the description for context (optional) like this: #898 (comment) and/or even quote the task description:
|
jorgeorpinel
left a comment
jorgeorpinel
left a comment
There was a problem hiding this comment.
Sorry for the delay, here's my feedback. Great so far but needs a little bit of polishing the details.
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
- Moved example to the bottom of all examples - Shortened YAML code output block - Explained YAML code concisely - Updated 4 semantic changes suggested by @jorgeorpinel - Shortened output of `import` command as suggested by @jorgeorpinel & @shcheklein
- Resolved `dvc update` in page link - Prettier formatting
Yep, you can fix the formatting manually or with Prettier. The docs contrib guide explains how to use it and set it up, in fact it should be automatic if you have installed the app locally. But please reach out to us on Discord if you're having troubles or questions regarding this.
I didn't understand this part, sorry. What are you trying to merge to what? |
Actually I was working on another issue locally so I thought it would get complicated to add and commit specific files only depending on issues. But I realize now that would still be way better than failing these tests.
This is regarding the duplicate PR #1354 of #1339 which had some instructions for the contributor to incorporate changes manually. I was stuck at this command in the instructions |
This comment has been minimized.
This comment has been minimized.
Ah yes, option no # 2 you should be able to do when there's a Restyled PR. That is to merge that automatic branch from the upstream back to your own branch. I'm curious why you weren't able to merge it, please share the commands and output next time if that happens. |
|
Another good one! Thanks @utkarshsingh99 I hope this helps you get closer to start planning your GSoD project idea application. |
3 participants
Additional, final fix #898
Specific addition: #898(comment)
Please check the link of the non-DVC git repository. I couldn't find any non-DVC repo in iterative that could provide for a dataset file. In case of any modifications, I would be happy to make changes to the link.
❗ 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. 🙏