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 summary for tuples #36276

Merged
merged 10 commits into from
Sep 11, 2020
Merged

Fix summary for tuples #36276

merged 10 commits into from
Sep 11, 2020

Conversation

briochemc
Copy link
Contributor

Removed sumary(io: IO, t::tuple) line as suggested in #36251 (comment).

@JeffBezanson
Copy link
Sponsor Member

Thanks. I think there are some tests of this to fix as well.

@StefanKarpinski
Copy link
Sponsor Member

The good news is you don't need to add tests, just fix the existing ones. Might be worth a note in NEWS.

Keno
Keno requested changes Jun 18, 2020
View reviewed changes
Copy link
Member

@Keno Keno left a comment

Choose a reason for hiding this comment

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

As per previous comments, tests need to be fixed (changing the review status drops it from my queue until comments are addressed).

@briochemc
Copy link
Contributor Author

tests need to be fixed

Yes, I'll try to do that early next week, sorry got very little time right now!

@briochemc
Copy link
Contributor Author

I just looked for summary in test/show.jl and changed those in the last commit (hopefully correctly)... But is that all that was needed?

@briochemc
Copy link
Contributor Author

I think I fixed the tests correctly. Please let me know if I need to do anything else!

@briochemc briochemc requested a review from Keno July 13, 2020 00:02
@JeffBezanson JeffBezanson added the status:merge me PR is reviewed. Merge when all tests are passing label Jul 19, 2020
@JeffBezanson
Copy link
Sponsor Member

Looks good, just needs a rebase now.

@briochemc
Copy link
Contributor Author

Again, I think I just did the correct thing by merging master back into the PR branch. Do I need to do anything else?

@KristofferC
Copy link
Sponsor Member

Looks like some test need fixing up.

JeffBezanson
JeffBezanson reviewed Aug 29, 2020
View reviewed changes
test/errorshow.jl Outdated Show resolved Hide resolved
@briochemc
Copy link
Contributor Author

(Gentle bump!)

@KristofferC KristofferC merged commit 9f6d4dd into JuliaLang:master Sep 11, 2020
@DilumAluthge DilumAluthge removed the status:merge me PR is reviewed. Merge when all tests are passing label Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JeffBezanson JeffBezanson JeffBezanson left review comments

@fredrikekre fredrikekre fredrikekre left review comments

@Keno Keno Awaiting requested review from Keno

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@briochemc @JeffBezanson @StefanKarpinski @KristofferC @Keno @fredrikekre @DilumAluthge