-
Notifications
You must be signed in to change notification settings - Fork 146
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
refactor!: remove second opts argument in Honeybadger.notify #499
base: master
Are you sure you want to change the base?
Conversation
This moves to keyword arguments exclusively. It's a breaking change because the following signature is no longer supported: Honeybadger.notify("test", {tags: 'testing, hash'})
👏 Kudos for the conventional commit PR naming style! With conventional commits, we can automate versioning, which will eventually help with automated releases. There's one minor correction: everything should be lowercase: Finally, I'm wondering if |
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.
Looks good to me. Sorry @joshuap for not waiting for your comment, otherwise my PR could have been this, of course.
I'm not sure refactor is it since refactoring shouldn't introduce any changes, right? |
Good point! Though, I usually prefer this definition, taken from here: One additional point is that usually automatic changelog generation will ignore |
@joshuap I confirmed that this is the case with our automated versioning and changelog generation on the ruby package. |
done! |
This moves to keyword arguments exclusively. See #426 for further context.
BREAKING CHANGE: The following signature is no longer supported:
Instead, you must do this: