Skip to content

Use StandardError over Exception #775

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use StandardError over Exception #775

wants to merge 1 commit into from

Conversation

denys281
Copy link

@denys281 denys281 mentioned this pull request Sep 16, 2019
Merged
8 tasks
@bquorning bquorning changed the title Use StandartError over Exception Use StandardError over Exception Nov 12, 2019
bquorning
bquorning reviewed Nov 12, 2019
View reviewed changes
README.adoc
@@ -1931,6 +1931,19 @@ end

Prefer the use of exceptions from the standard library over introducing new exception classes.

=== Do not `raise` `Exception` [[raise-exception]]

Use `StandardError` over `Exception`.
Copy link

Choose a reason for hiding this comment

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

Perhaps this section should include a sentence about why one should not raise Exception.

Copy link
Author

Choose a reason for hiding this comment

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

Could you help with wording?

Copy link
Member

Choose a reason for hiding this comment

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

@denys281 Would "[Exception] will bypass test frameworks and error reporters" do?

@denys281
Copy link
Author

@marcandre Could you take a look? Cop w merged some time ago.

@marcandre
Copy link
Contributor

This PR still needs work...
It's true one should not raise an Exception, but it is not true one should raise a StandardException; it's more an abstract base class than anything else.
One should raise the appropriate exception (e.g. TypeError) or if nothing fits, a RuntimeError (which shouldn't be specified).
Maybe it would be best to modify the "don't rescue Exception" section, as they are related... Don't raise one because you should not rescue those...

@pirj
Copy link
Member

pirj commented Jan 10, 2023

Ping @denys281

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@pirj pirj pirj left review comments

@bquorning bquorning bquorning left review comments

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

Successfully merging this pull request may close these issues.
4 participants
@denys281 @marcandre @pirj @bquorning