-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
👋 @bbrands02
Thank you for raising your pull request.
Please make sure you have followed our contributing guidelines. We will review it as soon as possible. In the meanwhile make sure your PR checks the following boxes
- Is based on an issue
- Has been locally tested
- Has been tested with the admin UI
- Has been discussed with the development team in an open channel
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.
Ome?
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.
In case he combines 2 messages he uses lower s and if this is the start of the message he uses capital S
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.
I would throw a 500-range here
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.
Same here, 400 range sounds like the test does something wrong, instead of that there's an error on the server
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.
docblock description incorrect
No description provided.