-
Notifications
You must be signed in to change notification settings - Fork 1
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
515/better error handling #529
Conversation
This comment has been minimized.
This comment has been minimized.
Fixes issue that caused logs not going to STDOUT due to some weird Meta-Programming-issues
This is now fixed by using a Formatter |
Thanks for the changes. It might be useful to prevent this behavior, by e.g.:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice changes! Another test for the error display might be helpful. (+ See my previous comment)
I opted for the counter version. I think its more transparent. |
…t2/vm-portal into 515/Better-error-handling
…rtal into 515/Better-error-handling
|
||
it 'does create notifications if the duplicates are one minute apart' do | ||
notify_user | ||
new_time = Time.now + 61 # seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "magic number" (i.e. the 60 seconds in the model) might be better suited for the AppSettings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #515