Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

Consolidate Debuggable.stackTrace with the rest of the error types #182

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Consolidate Debuggable.stackTrace with the rest of the error types #182

wants to merge 2 commits into from

Conversation

zdnk
Copy link

@zdnk zdnk commented Nov 19, 2018

References: vapor/vapor#1804

The stackTrace was optional and was unreachable by trying cast error: Error to Debuggable, it was always returning nil.

@joscdk joscdk requested a review from tanner0101 November 22, 2018 12:20
MrLotU
MrLotU approved these changes Mar 4, 2019
View reviewed changes
Copy link
Member

@MrLotU MrLotU left a comment

Choose a reason for hiding this comment

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

LGTM! @tanner0101 Could you also check?

tanner0101
tanner0101 suggested changes Mar 5, 2019
View reviewed changes
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

This is a breaking change unfortunately. And vapor/core is not a dependency of Vapor 4.

@MrLotU
Copy link
Member

MrLotU commented Mar 5, 2019

Ah. Of course. Something to keep in mind for Vapor 4 in that case.

@tonyarnold tonyarnold mentioned this pull request Oct 10, 2019
Closed
Base automatically changed from master to main March 12, 2021 02:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tanner0101 tanner0101 tanner0101 requested changes

@MrLotU MrLotU MrLotU approved these changes

At least 1 approving review is required to merge this pull request.

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

Successfully merging this pull request may close these issues.
3 participants
@zdnk @MrLotU @tanner0101