doc: Add windows example for Path.format#5700
doc: Add windows example for Path.format#5700mithun-daa wants to merge 3 commits intonodejs:masterfrom
Conversation
|
FYI: The path is not formatted if the // `root` will be used if `dir` is not specified and `name` + `ext` will be used
// if `base` is not specified
path.format({
root : "/",
ext : ".txt",
name : "file"
})
// returns
'/file.txt' |
doc/api/path.markdown
Outdated
There was a problem hiding this comment.
Minor nit... rather than \*nix:, perhaps just say An example on Posix systems:
|
LGTM with a nit |
|
Updated with the requested changes. |
|
LGTM |
1 similar comment
|
LGTM |
doc/api/path.markdown
Outdated
There was a problem hiding this comment.
This line should probably also be commented or moved to the end of the previous line.
There was a problem hiding this comment.
Sorry, I did not follow you @mscdex . Did you mean comment out the return value?
There was a problem hiding this comment.
@mithun-daa I would say no, or at least not until #5670 lands. It might now land. I think what you've done here is fine.
There was a problem hiding this comment.
It landed now, so I think it's best to update this PR to use the newer style.
There was a problem hiding this comment.
Pushed the requested changes.
Change the *nix to Posix systems
The return value of the windows example should be commented to conform to the system wide style
8d9963c to
0b6b6aa
Compare
PR-URL: #5700 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
|
Landed in 54e6a8d |
|
@jasnell Noob question - what do you mean by commits squashed? |
|
@mithun-daa ... the pull request contains 3 commits, I merged them into 1 |
|
Thanks for clearing that up @jasnell |
|
Thanks for stepping up @Mithun-data hope to see you more around :) |
|
Anytime @benjamingr - hope I can contribute more and hopefully more meaningful stuff. |
PR-URL: #5700 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Conflicts: doc/api/path.markdown
PR-URL: #5700 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: #5700 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Pull Request check-list
Please make sure to review and check all of these items:
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
test (or a benchmark) included?
existing APIs, or introduces new ones)?
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Affected core subsystem(s)
Please provide affected core subsystem(s) (like buffer, cluster, crypto, etc)
Description of change
Please provide a description of the change here.
Add windows sample for Path.format as requested in Issue #5671