-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Moved so many docs out of HelpDB. More examples. #18041
Conversation
297df7c
to
983b524
Compare
splice!(a::Vector, index::Integer, [replacement]) -> item | ||
|
||
Remove the item at the given index, and return the removed item. Subsequent items are | ||
shifted down to fill the resulting gap. If specified, replacement values from an ordered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which way is "down"?
Doctests again passed locally. |
cd(f::Function, dir::AbstractString=homedir()) | ||
|
||
Temporarily changes the current working directory and applies function `f` before returning. | ||
""" -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably simpler to put the docs that have OS-specific definitions after the if
blocks using the same syntax that was being used in the helpdb. Same with the other ones further down. That would avoid having to always check whether all the duplicates get updated when changes are made to one of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in latest. Anything else I should look after?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a couple more duplicated @doc
s that can get the same treatment as cd
, but other than that I didn't notice anything else out of place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still double-checking this, so may be good to wait for another set of eyes since backporting this might allow migration of the manual to be done in an automated way on both master and 0.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good to wait for another set of eyes
Yes, definitely.
might allow migration of the manual to be done in an automated way on both master and 0.5.
At the moment the translation scripts I'm using do seem to work fine against master
and 0.5
, though keeping things in sync as much as possible would be greatly appreciated.
@MichaelHatherly if you can point out specific spots I don't need to |
Sure, can do. Would you rather have inline comments here pointing out the spots, or I can make a quick diff of the changes that you can just apply? |
The diff option sounds like a way to learn something new, so let's go with that! |
https://gist.github.com/MichaelHatherly/cc9f1ce532e9f851ff6906815e517775 I've not yet compiled those changes since LLVM decided it was a good time to need recompiling... but I think the changes should be good. |
Thanks, @MichaelHatherly. |
5bd914c
to
dc30b82
Compare
This is impressive work! @tkelman What's the right strategy here? Merge and backport, or hold out until the 0.5 release is out and leave this on master? After this is merged, future doc improvements will be difficult to backport to 0.5, and perhaps for that reason, backport is desirable. Just thinking out aloud. |
Finish double-checking it then merge and backport. |
For functions that are just aliases of other functions, such as For example on this branch searching for docs on (This is probably worth mentioning in the docsystem docs. I'll write something up for the docs later today or tomorrow.) The aliases that should be changed are:
|
|
||
.. Docstring generated from Julia source | ||
|
||
Replace ``STDOUT`` by stream for all C and Julia level output to ``STDOUT``\ . Note that ``stream`` must be a TTY, a ``Pipe`` or a ``TCPSocket``\ . | ||
Create a pipe to which all C and Julia level :obj:`STDOUT` output will be redirected. Returns a tuple ``(rd, wr)`` representing the pipe ends. Data written to :obj:`STDOUT` may now be read from the ``rd`` end of the pipe. The ``wr`` end is given for convenience in case the old :obj:`STDOUT` object was cached by the user and needs to be replaced elsewhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate redirect_stdout
signatures here.
As mentioned in JuliaLang#18041 (comment) docstrings attached to aliases of functions should be avoided where possible.
As mentioned in #18041 (comment) docstrings attached to aliases of functions should be avoided where possible.
As mentioned in #18041 (comment) docstrings attached to aliases of functions should be avoided where possible. (cherry picked from commit 3ed55a4)
bba01c4
to
5131751
Compare
Doctests passed locally. I took CI skip off since I rebased. @MichaelHatherly is this ok assuming lights turn green? |
I only got a small way through reviewing this before backporting got in the way. planning on doing the rest tomorrow. |
""" | ||
readlink(path::AbstractString) -> AbstractString | ||
|
||
Returns the value of a symbolic link `path`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(readlink
docstring) is "the value of a symbolic link" unambiguous? maybe worth rewording to say something like "the target location a symbolic link points to" ?
|
||
.. Docstring generated from Julia source | ||
|
||
Display an informational message. Argument ``msg`` is a string describing the information to be displayed. | ||
Display an informational message. Argument ``msg`` is a string describing the information to be displayed. The ``prefix`` kwarg can be used to override the default prepending of ``msg``\ . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(info
docstring) spell out kwarg
?
@tkelman thoughts? |
Let's get it merged. Did you want to do a rebase to clean up the history? |
Will do when I get home On Sat, Sep 3, 2016, 8:38 AM Tony Kelman [email protected] wrote:
|
269a924
to
b4c6f59
Compare
do the honors? |
🎉 |
As mentioned in JuliaLang#18041 (comment) docstrings attached to aliases of functions should be avoided where possible.
I'm sorry this is so huge. Once I started, I couldn't stop. Doctests passed locally.