Skip to content

Conversation

@ararslan
Copy link
Member

Needed for JuliaLang/julia#25959. In that PR, the lowercase names stdin, stdout, and stderr will be claimed by Base, so this avoids overwriting those names, which should hopefully allow documentation to build on that PR.

@ararslan ararslan requested a review from mortenpi February 16, 2018 06:03
src/DocChecks.jl Outdated
value :: Any # The value returned when evaluating `input`.
hide :: Bool # Semi-colon suppressing the output?
stdout :: IOBuffer # Redirected STDOUT/STDERR gets sent here.
io :: IOBuffer # Redirected STDOUT/STDERR gets sent here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be necessary to change field names.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, though arguably this is a better name anyway. ¯\_(ツ)_/¯ I can change it back if that'd be preferable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that stdout is a little bit more specific/informative w.r.t. the purpose of that field, although either has pros and cons. As such, since we don't have to make this change, maybe keep it as stdout for now?

@fredrikekre
Copy link
Member

The failure on nightly is fixed in 31044e5

Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, LGTM! Will merge and tag after #643 if there are no objections.

src/DocChecks.jl Outdated
value :: Any # The value returned when evaluating `input`.
hide :: Bool # Semi-colon suppressing the output?
stdout :: IOBuffer # Redirected STDOUT/STDERR gets sent here.
io :: IOBuffer # Redirected STDOUT/STDERR gets sent here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that stdout is a little bit more specific/informative w.r.t. the purpose of that field, although either has pros and cons. As such, since we don't have to make this change, maybe keep it as stdout for now?

@mortenpi mortenpi added this to the 0.13.2 milestone Feb 16, 2018
@ararslan
Copy link
Member Author

Thanks for restarting CI, @mortenpi! Could you merge and tag?

@mortenpi mortenpi merged commit d45e59e into master Feb 18, 2018
@mortenpi mortenpi deleted the aa/stdout branch February 18, 2018 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants