-
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
V2 #7
V2 #7
Conversation
def update_connection(connection_id, body) | ||
fail Auth0::MissingConnectionId, 'you must specify a connection id' if connection_id.to_s.empty? | ||
fail Auth0::InvalidParameter, 'Must supply a valid connection id' if connection_id.to_s.empty? |
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 format does not match on all validations for the rest of the endpoints. What form should we use: "you must..." or "Must..."?
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.
Most of the errors are 'Must....;, so we'll keep this one, but we should change it in clients.rb
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.
Changed
Change email path variable name
def create_client(name, options = {}) | ||
fail Auth0::MissingParameter, 'you must specify a valid client name' if name.to_s.empty? |
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 should be 'Must...' and all the message in this class too.
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.
BTW, we should also update the tests.
def patch_user(user_id, options) | ||
fail Auth0::MissingUserId, 'Must supply a valid user_id' if user_id.to_s.empty? | ||
fail Auth0::InvalidParameter, 'Must supply a valid body' if options.to_s.empty? |
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.
Message says Must supply body and parameter is options. Is this message set by requirement?
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.
Good Catch! Changed
All feedback applied. Merging. |
No description provided.