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

add some global error handling options #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zvkemp
Copy link
Contributor

@zvkemp zvkemp commented Oct 9, 2018

We probably don't need to both report an error back to Slack and re-raise it (depending on the ActiveJob backend config, this may result in the same failed command being run multiple times). This sets the default to only printing the error message.

})

raise e
if Slackathon.report_errors?
Copy link
Member

Choose a reason for hiding this comment

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

What about echo_errors?

@@ -17,4 +17,21 @@ def self.queue
def self.queue=(queue)
@queue = queue
end

def self.report_errors?
@report_errors = true unless defined?(@report_errors)
Copy link
Member

Choose a reason for hiding this comment

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

We should probably just change this file to eagerly assign the default values right after we define the setter. Do you think that would break anything?

@wagenet
Copy link
Contributor

wagenet commented Jan 22, 2020

@chancancode should we just merge this?

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

3 participants