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

Don't use std(in|out|err) as identifiers #641

Merged
merged 2 commits into from
Feb 18, 2018
Merged

Don't use std(in|out|err) as identifiers #641

merged 2 commits into from
Feb 18, 2018

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
@@ -172,7 +172,7 @@ mutable struct Result
file :: String # File in which the doctest is written. Either `.md` or `.jl`.
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
@@ -172,7 +172,7 @@ mutable struct Result
file :: String # File in which the doctest is written. Either `.md` or `.jl`.
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.

4 participants