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

Use logging system for doctest-failure reporting. #958

Merged
merged 1 commit into from
Feb 14, 2019

Conversation

fredrikekre
Copy link
Member

No description provided.

Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

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

LGTM! Just a bunch of bikeshedding comments, feel free to disregard them 🙂

src/DocTests.jl Outdated Show resolved Hide resolved
src/DocTests.jl Show resolved Hide resolved
src/DocTests.jl Show resolved Hide resolved
src/DocTests.jl Show resolved Hide resolved
src/Utilities/TextDiff.jl Show resolved Hide resolved
@fredrikekre fredrikekre merged commit 951f577 into master Feb 14, 2019
@fredrikekre fredrikekre deleted the fe/doctest-failure branch February 14, 2019 09:57
mortenpi added a commit that referenced this pull request May 8, 2019
The issue was introduced in #958, where the push!, which was hidden away
in the DocTests.report function, got accidentally removed. It is not
clear from its name that the report function should have a side-effect
like this, so it is cleaner if the push! calls are next to the report
calls, even if it creates slight code duplication.

Fix #1003
mortenpi added a commit that referenced this pull request May 8, 2019
The issue was introduced in #958, where the push!, which was hidden away
in the DocTests.report function, got accidentally removed. It is not
clear from its name that the report function should have a side-effect
like this, so it is cleaner if the push! calls are next to the report
calls, even if it creates slight code duplication.

Fix #1003
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants