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

show stack trace in verbose mode (closes #2480) #2481

Merged
merged 4 commits into from Jul 5, 2017

Conversation

Projects
None yet
3 participants
@Stift
Contributor

Stift commented Jul 4, 2017

minimize the output in normal mode when getPackageDetails have a problem.
See #2480 for more info.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki
Member

forki commented Jul 4, 2017

/cc @matthid

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Jul 4, 2017

Member

Oh I was too slow: #2480 (comment)
Please only handle the one particular instance of the error.

Member

matthid commented Jul 4, 2017

Oh I was too slow: #2480 (comment)
Please only handle the one particular instance of the error.

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Jul 4, 2017

Member

The reasoning is that I want to move away from all the general "with e ->" clauses thoughout the code base. but that is a painful process (so I kind of provoke people to open issues by printing stacktraces on errors, this was imho better than if I forgot something and introduced breaking changes)

Member

matthid commented Jul 4, 2017

The reasoning is that I want to move away from all the general "with e ->" clauses thoughout the code base. but that is a painful process (so I kind of provoke people to open issues by printing stacktraces on errors, this was imho better than if I forgot something and introduced breaking changes)

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Jul 4, 2017

Contributor

I'll understand.
Updated it and limiting to IOException. Unfortunately a bunch of inherited classes are also now catched the same way (PathTooLong, etc.). Is this okay?

Contributor

Stift commented Jul 4, 2017

I'll understand.
Updated it and limiting to IOException. Unfortunately a bunch of inherited classes are also now catched the same way (PathTooLong, etc.). Is this okay?

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Jul 4, 2017

Member

@Stift in a perfect world i'd like to only catch the access denied error, but I can see the difficulties here. So I'm fine with it. Maybe we can add a code comment to clarify why it was added (to make it easier for future contributors to handle the sub-types differently). A link to your issue is probably just fine.

Member

matthid commented Jul 4, 2017

@Stift in a perfect world i'd like to only catch the access denied error, but I can see the difficulties here. So I'm fine with it. Maybe we can add a code comment to clarify why it was added (to make it easier for future contributors to handle the sub-types differently). A link to your issue is probably just fine.

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Jul 4, 2017

Contributor

I know, that perfect world I also dream of. I just wanted to prevent filtering on message strings and the exception does not give more details.

Contributor

Stift commented Jul 4, 2017

I know, that perfect world I also dream of. I just wanted to prevent filtering on message strings and the exception does not give more details.

@matthid

matthid approved these changes Jul 4, 2017

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Jul 4, 2017

Member

Yes I agree, filtering of the string is error prone. Thanks for looking into this.

Member

matthid commented Jul 4, 2017

Yes I agree, filtering of the string is error prone. Thanks for looking into this.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jul 5, 2017

Member

thanks!

Member

forki commented Jul 5, 2017

thanks!

@forki forki merged commit bb7caf0 into fsprojects:master Jul 5, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Stift Stift deleted the Stift:fixVerboseOutput_ branch Jul 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment