Navigation Menu

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

Fixed filter log statement to allow for resque job.rb patching. #1551

Closed
wants to merge 2 commits into from

Conversation

tmedford
Copy link

@tmedford tmedford commented Mar 9, 2017

Removed lib dependency. Causes error when gem/lib/resque/job.rb where paths like ./lib/extensions/resque/job.rb are in play

… paths like ./lib/extensions/resque/job.rb are in play
@coveralls
Copy link

coveralls commented Mar 9, 2017

Coverage Status

Coverage remained the same at 82.863% when pulling 50638aa on tmedford:master into 51e6265 on resque:master.

@tmedford
Copy link
Author

Any update

1 similar comment
@tmedford
Copy link
Author

tmedford commented Apr 7, 2017

Any update

@corincerami
Copy link
Member

This seems reasonable enough to me, although I don't entirely understand the issue it's solving. Could you just please add an entry to HISTORY.md in the Unreleased section to describe your fix? Thanks.

@coveralls
Copy link

coveralls commented Apr 19, 2017

Coverage Status

Coverage remained the same at 83.368% when pulling 8bde661 on tmedford:master into 374a753 on resque:master.

Copy link
Contributor

@jeremy jeremy left a comment

Choose a reason for hiding this comment

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

This would indeed clean lib/myextension/etc/resque/job.rb, but to anyone reading the code it'd look like a lucky accident.

If we want to support other backtrace cleaning approaches, let's do it in a clear, direct manner, like taking a backtrace_cleaner: … in Resque::Failure::Base.

@jeremy jeremy closed this Feb 14, 2018
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