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 displayException instead of show for showing exceptions #330

Merged
merged 2 commits into from Jul 2, 2023

Conversation

adamgundry
Copy link
Collaborator

Here's my proposed resolution to #327. By calling displayException instead of show we get prettier exception output for any exception type that implements displayException, including tasty's version of HUnitFailure. (Unfortuntately HUnit's version of HUnitFailure doesn't currently implement displayException, but that's a HUnit issue.)

@andreasabel
Copy link
Collaborator

Why doesn't this run through CI?

@adamgundry
Copy link
Collaborator Author

I'm not sure if it is to do with my having opened a branch from a fork? Or is it perhaps that Travis CI now requires applying for credits for open source projects?

@andreasabel
Copy link
Collaborator

Yeah, Travis is dead, but there is a GHA CI now: https://github.com/UnkindPartition/tasty/blob/16289a77495eb8279c5e544886ef52503becd148/.github/workflows/ci.yaml
Maybe it is misconfigured, it says on: [push] where I usually see on: [push, pull_request] or some refinement of this.
Ping @UnkindPartition.

@andreasabel
Copy link
Collaborator

@adamgundry : I updated the CI triggers. Can you please rebase this onto master and force-push, to see if CI runs now?

@adamgundry
Copy link
Collaborator Author

Thanks @andreasabel, looks like it is indeed running now.

@Bodigrim
Copy link
Collaborator

@andreasabel @VictorCMiraldo @martijnbastiaan this looks good to me, any more opinions?

Copy link
Contributor

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

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

LGTM. A changelog entry would make sense.

Copy link
Collaborator

@VictorCMiraldo VictorCMiraldo left a comment

Choose a reason for hiding this comment

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

LGTM to me too!

@Bodigrim Bodigrim merged commit 804b2a7 into UnkindPartition:master Jul 2, 2023
12 checks passed
@Bodigrim
Copy link
Collaborator

Bodigrim commented Jul 2, 2023

I've updated changelogs, rebased and merged. Thanks, @adamgundry!

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

Successfully merging this pull request may close these issues.

None yet

5 participants