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.

Sign up for GitHub

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

Jump to bottom

Add documentation for Base.Order module #36886

Merged
merged 3 commits into from
Sep 29, 2020
Merged

Add documentation for Base.Order module #36886

merged 3 commits into from
Sep 29, 2020

Conversation

jlumpe
Copy link
Contributor

@jlumpe jlumpe commented Aug 3, 2020 •
edited
Loading

Adds docstrings to Base.Order module and adds a section to the "Sorting and Related Functions" documentation page. Resolves #18370.

@jlumpe jlumpe force-pushed the ordering-docs branch 2 times, most recently from 1589ae9 to 8a7c3d9 Compare August 3, 2020 19:31
@jlumpe jlumpe changed the title Add docstrings for Base.Order module Add documentation for Base.Order module Aug 3, 2020
@jlumpe
Copy link
Contributor Author

jlumpe commented Aug 3, 2020

I did not add a docstring to the ordtype function because I can't actually tell what it does, but it seems like it is meant to be deprecated in the future anyway.

@jlumpe jlumpe force-pushed the ordering-docs branch 2 times, most recently from c6c1944 to aac49e7 Compare August 4, 2020 03:22
@jlumpe
Copy link
Contributor Author

jlumpe commented Sep 1, 2020

Any chance of getting this reviewed?

@jlumpe
Copy link
Contributor Author

jlumpe commented Sep 14, 2020

Bump?

nalimilan
nalimilan reviewed Sep 24, 2020
View reviewed changes
Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

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

Thanks for this! Looks good apart from a few details.

base/ordering.jl Show resolved Hide resolved
base/ordering.jl Outdated Show resolved Hide resolved
base/ordering.jl Show resolved Hide resolved
base/ordering.jl Show resolved Hide resolved
base/ordering.jl Outdated Show resolved Hide resolved
base/ordering.jl Outdated Show resolved Hide resolved
base/ordering.jl Outdated Show resolved Hide resolved
base/ordering.jl Outdated Show resolved Hide resolved
@nalimilan nalimilan added the docs This change adds or pertains to documentation label Sep 24, 2020
@jlumpe
Copy link
Contributor Author

jlumpe commented Sep 24, 2020

Thanks for the review! I have committed your suggestions.

@StefanKarpinski
Copy link
Member

I've restarted the two failing CI runs, but it seems fairly unlikely that this change could possibly be causing failures.

@jlumpe
Copy link
Contributor Author

jlumpe commented Sep 27, 2020

It seems that one succeeded but the other is still failing. I'm not sure how to interpret the output of the failed one, but I agree this doesn't seem like it should break anything (especially garbage collection, which is what I think this test is?).

@StefanKarpinski
Copy link
Member

I'm going to assume it can't possibly be relevant.

@StefanKarpinski StefanKarpinski merged commit be54a6c into JuliaLang:master Sep 29, 2020
@jlumpe jlumpe deleted the ordering-docs branch March 22, 2021 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nalimilan nalimilan nalimilan left review comments

Assignees
No one assigned
Labels
docs This change adds or pertains to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No documentation for Base.Order and Ordering for sort and search functions
3 participants
@jlumpe @StefanKarpinski @nalimilan