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

doc,fs: document WHATWG URL support #12341

Closed
addaleax opened this issue Apr 11, 2017 · 7 comments
Closed

doc,fs: document WHATWG URL support #12341

addaleax opened this issue Apr 11, 2017 · 7 comments
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@addaleax
Copy link
Member

addaleax commented Apr 11, 2017

  • Version: >= 7.6.0
  • Platform: n/a
  • Subsystem: fs, url

#10739 (fs: allow WHATWG URL and file: URLs as paths) was landed without documentation, because url.URL itself was undocumented at that time. Now url.URL is documented here, so the fs docs should include its support, too.

I think this is about what needs to be done:

  • URL needs to be added to the lists of supported types for the filename parameters
  • It needs to be mentioned that only file:/// URL objects are supported
  • An entry needs to be added to the changes: lists for the relevant methods that states the version this was added in (7.6.0) and that the support is currently still experimental.

As always, let us know here, in #node-dev on Freenode, or basically anywhere where you can reach us if you need any help or have any questions!

(oh, also: please comment here if you would like to try and take this on, so people don’t get in each other’s way. 😸 )

@addaleax addaleax added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Apr 11, 2017
@jasnell
Copy link
Member

jasnell commented Apr 12, 2017

I'm happy to help as a mentor on this for those that pick it up.

@anchnk
Copy link

anchnk commented Apr 12, 2017

If that can wait until saturday i would love to give it a try as a first contribution. You guys deserve all of our freetime as support. I wanted to start contributing to node since some time now. That seems to be a good first step

@jasnell
Copy link
Member

jasnell commented Apr 19, 2017

@anchnk sorry just saw this. Feel free to dm me on twitter for specific questions on getting started

@anchnk
Copy link

anchnk commented Apr 25, 2017

It needs to be mentioned that only file:/// URL objects are supported

For that point, I am curious what would be the recommended way to handle this ?

I see two options :

  • Every updated method/class mentions that statement.
  • It's stated once in the documentation and every updated method/class refers to it.

I would like to stay in line, maybe it's something you already sort in the past.

@TimothyGu
Copy link
Member

@anchnk I'd prefer it stated once, much like the existing Buffer API section.

@anchnk
Copy link

anchnk commented Apr 26, 2017

@TimothyGu Thank you for your feedback ! I should put a note mentioning the restriction at the root level of the module's description then ?

@jasnell
Copy link
Member

jasnell commented Apr 26, 2017

That would work. I tend to prefer the documentation closer to the actual methods to keep users from having to jump around through the document to find all the bits they need. But, reducing duplication is a good thing also, and as @TimothyGu points out, there is plenty of precedence. A note with the restriction at the root level sounds just fine.

anchnk pushed a commit to anchnk/node that referenced this issue May 4, 2017
Update fs module documentation adding WHATWG file URLS support for
relevant fs functions/classes.

Fixes: nodejs#12341
Refs: nodejs#10739
@jasnell jasnell closed this as completed in c1b3b95 May 4, 2017
anchnk pushed a commit to anchnk/node that referenced this issue May 6, 2017
Update fs module documentation adding WHATWG file URLS support for
relevant fs functions/classes.

Fixes: nodejs#12341
Refs: nodejs#10739
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

No branches or pull requests

4 participants