-
Notifications
You must be signed in to change notification settings - Fork 501
Don't use std(in|out|err) as identifiers #641
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
Conversation
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. |
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.
It shouldn't be necessary to change field names.
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.
Fair point, though arguably this is a better name anyway. ¯\_(ツ)_/¯ I can change it back if that'd be preferable.
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 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?
|
The failure on nightly is fixed in 31044e5 |
mortenpi
left a comment
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.
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. |
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 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?
|
Thanks for restarting CI, @mortenpi! Could you merge and tag? |
Needed for JuliaLang/julia#25959. In that PR, the lowercase names
stdin,stdout, andstderrwill be claimed by Base, so this avoids overwriting those names, which should hopefully allow documentation to build on that PR.