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

Missing.md updated #42500

Merged
merged 5 commits into from
Oct 20, 2021
Merged

Missing.md updated #42500

merged 5 commits into from
Oct 20, 2021

Conversation

mishmanners
Copy link
Contributor

Typo fixes, paragraph fixes, grammar usage, and "massage" of text.

Hopefully that makes it much easier to read and understand 😄

@KristofferC
Copy link
Member

KristofferC commented Oct 5, 2021

Welcome to Julia and thanks for a nice first contribution!

The removal of most of the line breaks makes it a little bit hard to see from the diff what the change are.
The contribution guide suggests a 92 character line length limit and I think that also applies to the documentation source files.

Note also the failed whitespace CI: https://buildkite.com/julialang/julia-master/builds/4358#46bbefc3-53c5-4e39-b60e-a52129586aad

doc/src/manual/missing.md:28:
  | Error: trailing whitespace found in source file(s)
  |  
  | This can often be fixed with:
  | git rebase --whitespace=fix HEAD~1
  | or
  | git rebase --whitespace=fix master
  | and then a forced push of the correct branch

@mishmanners
Copy link
Contributor Author

Oh I didn't remove any of the line breaks, just made it so things aren't squashed onto one side of the screen. Lots of updates, typos, and grammar fixes, but I'll fix the failed whitespace.

@kshyatt kshyatt added the docs This change adds or pertains to documentation label Oct 5, 2021
@KristofferC
Copy link
Member

just made it so things aren't squashed onto one side of the screen

I guess that is what I meant with

The contribution guide suggests a 92 character line length limit and I think that also applies to the documentation source files.

@mishmanners
Copy link
Contributor Author

mishmanners commented Oct 7, 2021

Ahh right. All good. Did you figure out what broke @KristofferC ? I couldn't find any differences in the lines that threw the errors.

@ViralBShah
Copy link
Member

Would it be safe to merge this since tests are passing on linux64?

doc/src/manual/missing.md Outdated Show resolved Hide resolved
doc/src/manual/missing.md Outdated Show resolved Hide resolved
Co-authored-by: woclass <git@wo-class.cn>
Copy link
Contributor Author

@mishmanners mishmanners left a comment

Choose a reason for hiding this comment

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

LGTM.

which implies that we do not know how the program should behave. A [`TypeError`](@ref)
is thrown as soon as a `missing` value is encountered in this context
Control flow operators including [`if`](@ref), [`while`](@ref) and the [ternary operator](@ref man-conditional-evaluation) `x ? y : z` do not allow for missing values. This is because of the uncertainty about whether the actual value would be `true` or `false` if we could observe it. This implies we do not know how the program should behave. Thus, a [`TypeError`](@ref) is thrown as soon as a `missing` value is encountered in this context:

```jldoctest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where the error is and there's no change from the original. Not sure why it's failing @KristofferC

which implies that we do not know how the program should behave. A [`TypeError`](@ref)
is thrown as soon as a `missing` value is encountered in this context
Control flow operators including [`if`](@ref), [`while`](@ref) and the [ternary operator](@ref man-conditional-evaluation) `x ? y : z` do not allow for missing values. This is because of the uncertainty about whether the actual value would be `true` or `false` if we could observe it. This implies we do not know how the program should behave. Thus, a [`TypeError`](@ref) is thrown as soon as a `missing` value is encountered in this context:

```jldoctest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically these lines @KristofferC . Thoughts?

@mishmanners
Copy link
Contributor Author

@KristofferC let me know if there's anything else I need to do to help get this merged.

Copy link
Member

@KristofferC KristofferC left a comment

Choose a reason for hiding this comment

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

Looks good now after the fixups by @fredrikekre. (If you look at the diff now vs the one before you can see how much easier it is to review now).

@fredrikekre fredrikekre merged commit 05515f4 into JuliaLang:master Oct 20, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants