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

Format spec results with pretty inspect #11635

Conversation

JamesGood626
Copy link
Contributor

Changing all instances of Object.inspect with Object.pretty_inspect in the /src/spec/expectations.cr file for better readability of classes and structs in spec test results.

https://forum.crystal-lang.org/t/formatting-spec-test-results/4129/3

JamesGood626 and others added 3 commits December 21, 2021 08:48
… with Object.pretty_inspect to facilitate formatting of classes and structs in the displayed results after running a spec test suite.
… with Object.pretty_inspect to facilitate formatting of classes and structs in the displayed results after running a spec test suite.
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

LGTM.

Let's merge it and see how it works out. If we don't like the format, we can revert before the next release.

@beta-ziliani beta-ziliani added this to the 1.7.0 milestone Dec 20, 2022
@straight-shoota
Copy link
Member

@beta-ziliani Actually, I'd rather postpone this to 1.8. The idea was to merge this in advance of a release so we have time to observe the effects.

@beta-ziliani beta-ziliani modified the milestones: 1.7.0, 1.8.0 Dec 21, 2022
@straight-shoota straight-shoota merged commit 084e734 into crystal-lang:master Jan 11, 2023
@straight-shoota
Copy link
Member

We're nearing the 1.8 release, so I'm just checking back on this. Nobody has complained about the pretty_print change. And honestly, I haven't explicitly noticed any difference in spec behaviour. So I think we're good with rolling this out 👍

This pull request was closed.
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.

3 participants