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

remove some links to Nullable and methods #19961

Merged
merged 3 commits into from
Jan 18, 2017
Merged

remove some links to Nullable and methods #19961

merged 3 commits into from
Jan 18, 2017

Conversation

mzaffalon
Copy link
Contributor

@mzaffalon mzaffalon commented Jan 10, 2017

I find it distracting that every word Nullable and every method associated is a link.
The link should appear only the first time a new method is mentioned.

I find it distracting that every word `Nullable` and every method associated is a link.
The link should appear only the first time the new method is mentioned.
@nalimilan nalimilan added missing data Base.missing and related functionality docs This change adds or pertains to documentation labels Jan 10, 2017
@tkelman tkelman requested a review from kshyatt January 10, 2017 13:38
@waldyrious
Copy link
Contributor

@mzaffalon could you add the link in the first paragraph of the Nullable types section? Here would probably be the appropriate place:

Julia provides a parametric type called Nullable{T}

itself was missing, or the test failed.
* Perform general operations on single or multiple [`Nullable`](@ref)
* Perform general operations on single or multiple `Nullable`
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the words or multiple here; I believe they were left over from #19787.


[`broadcast`](@ref) can be thought of as a way to make existing operations work
`broadcast` can be thought of as a way to make existing operations work
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "multiple data" mention here.

Copy link
Contributor Author

@mzaffalon mzaffalon Jan 11, 2017

Choose a reason for hiding this comment

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

You mean "broadcast can be thought of as a way to make existing operations work simultaneously and propagate nulls"? Can you please confirm the sentence has a meaning?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps "make existing operations work on Nullable arguments and propagate nulls"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TotalVerb can you please check whether the change is OK? Otherwise feel free to edit it.

Copy link
Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

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

Yes, only the first reference to something in a section (chapter, whatever is a reasonable unit of text) should be a link to the definition.

@mzaffalon
Copy link
Contributor Author

Can this PR be merged?

@tkelman tkelman merged commit b8971ea into JuliaLang:master Jan 18, 2017
@tkelman
Copy link
Contributor

tkelman commented Jan 18, 2017

Yep, thanks for the reminder

@mzaffalon mzaffalon deleted the patch-1 branch January 18, 2017 09:11
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 missing data Base.missing and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants