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

fix remaining doctest failures and enable doctests on CI #27000

Merged
merged 3 commits into from
May 17, 2018
Merged

fix remaining doctest failures and enable doctests on CI #27000

merged 3 commits into from
May 17, 2018

Conversation

fredrikekre
Copy link
Member

@fredrikekre fredrikekre commented May 5, 2018 •
edited
Loading

This fixes all remaining doc(test) failures, and enables doctests on Travis Linux 64-bit (fix #19528). On my machine this changes the time for the doc build from ~2 to ~5 minutes, but I think it is well worth it, because, as you can see from the diff, we keep deprecating our own manual.

ATM this includes #26973 and #26993, the relevant changes to review here is 3c29c7b + 14f08c1

@fredrikekre fredrikekre added the docs This change adds or pertains to documentation label May 5, 2018
@fredrikekre fredrikekre force-pushed the fe/enable-doctests branch from 14f08c1 to e955114 Compare May 8, 2018 10:21
@fredrikekre
Copy link
Member Author

Unrelated download failure on AV 32-bit, will merge tomorrow unless someone objects.

@fredrikekre fredrikekre mentioned this pull request May 9, 2018
Merged
nalimilan
nalimilan reviewed May 9, 2018
View reviewed changes
doc/src/manual/arrays.md Outdated
@@ -392,8 +392,10 @@ julia> x = collect(reshape(1:9, 3, 3))
2 5 8
3 6 9

julia> x[1:2, 2:3] = -1
-1
julia> x[1:2, 2:3] .= -1
Copy link
Member

Choose a reason for hiding this comment

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

A few lines above we say we're going to show that = is lowered to setindex!: the text should somewhat be adapted to explain why one needs to use .= here. Maybe better show a single-element assignment first.

@KristofferC
Copy link
Member

Bump

@fredrikekre
Copy link
Member Author

I have this rebased locally but I think we should tweak some things. As it is now we build the docs as part of make install, i.e. before the test suite. I think we should run the doctests only after the tests since it would be annoying if you have to worry about the docs before seeing the test results (for e.g. wip branches). I will push an updated version of this tomorrow.

@fredrikekre fredrikekre force-pushed the fe/enable-doctests branch 4 times, most recently from a4e600c to 1c447d4 Compare May 14, 2018 13:26
@fredrikekre
Copy link
Member Author

If anyone is attempted to merge due to the all green CI, PLEASE DON'T! This PR fails rebased to master due to #27103

@fredrikekre fredrikekre changed the title fix remaining doctest failures and enable doctests on CI [DON'T MERGE] fix remaining doctest failures and enable doctests on CI May 15, 2018
@fredrikekre fredrikekre force-pushed the fe/enable-doctests branch from 1c447d4 to 799f8b0 Compare May 17, 2018 06:47
@fredrikekre fredrikekre changed the title [DON'T MERGE] fix remaining doctest failures and enable doctests on CI fix remaining doctest failures and enable doctests on CI May 17, 2018
@fredrikekre
Copy link
Member Author

Disabled the failing test for now. Will merge if CI passes.

@fredrikekre fredrikekre merged commit 406615f into master May 17, 2018
@fredrikekre fredrikekre deleted the fe/enable-doctests branch May 17, 2018 09:51
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.

Re-enable doctests
3 participants
@fredrikekre @KristofferC @nalimilan