suggest some minor things to API

[shcheklein]

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. 🙏

[jorgeorpinel]

Agee about removing the var name but its not very obvious what the comment below is (and probably creates a horizontal scroll). Not sure it's needed for basic usage.

[shcheklein]

I think it's intuitively clear ... we can even bring var name back in the comment ... we need to show something than nothing

we can just add

`# returns 'http://....'

[jorgeorpinel]

I don't think the basic usage block needs to show the variable value. That's what examples are for I think.

But if you want to keep this I would not reintroduce var name, maybe just make the comment shorter? # https://remote.dvc.org/dataset-registry/a3/04a... (maybe in the example BTW, so that no scrolls are shown) but not a huge deal about the scrolls for these cases.

[shcheklein]

I think it makes this example significantly better if we show the value, I would even say that this the most important part - you see right away what it does

I would keep at as simple as possible, would add returns at most

[jorgeorpinel]

OK. Actually I would reintroduce the var name now that I think about it. You wouldn't call a string returning function without assigning it to something. So my full suggestion is:

resource_url = dvc.api.get_url(
    'get-started/data.xml',
    repo='https://github.com/iterative/dataset-registry')
    
# resource_url = https://remote.dvc.org/dataset-registry/a3/04a...

[jorgeorpinel]

Or wrap in print() and say # prints http... in the comment.

[shcheklein]

don't use = though, write it in a plain language - resource_url is now 'http:///'

[shcheklein]

otherwise it looks like a commented code

[jorgeorpinel]

OK to move it but the original text is a little better I think, because it doesn't imply that the .dir file will exist (and contain "the mapping of files...").

[shcheklein]

not sure I got the point ...

[shcheklein]

I just wanted to avoid to many details (like .dir, etc - it's internal stuff)

[jorgeorpinel]

I think mentioning the .dir ending is appropriate as it may be something the user notices and wonders about when using this fn.

Suggested change

	If the target is a directory, the returned URL will point to a file that
	contains the mapping of files in the directory (as a JSON array), along with
	their hash values. Refer to
	[Structure of cache directory](/doc/user-guide/dvc-files-and-directories#structure-of-cache-directory)
	and `dvc add` to learn more about how DVC handles data directories.
	⚠️ This function does not check for the actual existence of the file or
	directory in the remote storage.
	If the target is a directory, the returned URL will end in `.dir`: a file that
	contains the mapping of files in the directory. Refer to
	[Structure of cache directory](/doc/user-guide/dvc-files-and-directories#structure-of-cache-directory)
	and `dvc add` to learn more about how DVC handles data directories.
	⚠️ This function does not check for the actual existence of the file or
	directory in the remote storage.

[shcheklein]

they should not care about such specifics, we might change schema for those URL ... we can mention something like (ends with .dir) - but that should be enough

[jorgeorpinel]

we can mention something like (ends with .dir) - but that should be enough

Good idea. Keeping the last sentence too (link to dvc dir struct), I assume.

[jorgeorpinel]

This is repetitive. Maybe just something like "Prints the URL string value if we run it from terminal:" ?

[shcheklein]

Again, it's easier to read. Repetition not a big problem. Code snippet just illustrates the code (and gives a command to run). I would keep it extra explicit. Up yo you again. Don't have a strong opinion

[jorgeorpinel]

Repetition not a big problem.

Yeah but why do it if it's not needed? Just keeping the top paragraph and end it in "if we run it.", and removing the console block code should be enough I think.

p.s. The original one I did with the --- hr was also kind of obvious, like in the basic usage comment 🙂 and I like how it looked rendered.

[jorgeorpinel]

I'm going to merge this and address the remaining stuff in the api branch (#908 PR).

[jorgeorpinel]

Done! In 8089423.