-
Notifications
You must be signed in to change notification settings - Fork 31
Conversation
Add arguments property
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.
Hi @jkeane64
I took a look at your requested change and it looked good. Only one small change to maintain consistency with the rest of our models. If you don't mind making that change, I can work on making a release.
Thanks,
Ben
src/models/Fault.js
Outdated
@@ -62,6 +68,9 @@ export default class Fault { | |||
if (data.hasOwnProperty('type')) { | |||
obj['type'] = ApiClient.convertToType(data['type'], 'String') | |||
} | |||
if (data.hasOwnProperty('arguments')) { | |||
obj['arguments'] = ApiClient.convertToType(data['arguments'], Object) |
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.
If we are going to by consistent in the way our type coercion is done in the rest of the models the correct argument to pass into convertToType
should be {String: 'Blob'}
or {String: Object}
. I would try these to ensure they work, but looking at the code it should work no problem. Doing it this way is a little bit more descriptive on the data's property type.
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.
Done 👍
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.
👍
Add arguments property
Description
Adds arguments to the Fault model as this provides valuable information specifically for Reitmans project
No access to arguments object from fault message
Type of change
Changes
Added arguments to Fault Model
How to test this PR?
Checklist:
npm run lint
)README.md
andCHANGELOG.md
) - N/Anpm test
)