-
Notifications
You must be signed in to change notification settings - Fork 50
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
Replaces old log output format with the new mint log style #117
Conversation
@nitisht @ebozduman travis CI fails on npm install. I guess its not related to this PR. We would need to fix this problem before merging this PR |
I was looking into the failure. Here are the logged error messages from travis:
|
The failure was a Travis CI issue, I restarted the build on Travis and it built fine. |
args: args_arr, # array of arg names. This'll be replaced with a | ||
# hash map, arg/value pairs insdie the caller method | ||
comment: comment, | ||
message: nil, |
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.
can you add the duration
section as well?
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.
Will do.
Nitish, |
e70babd
to
b5d5d9e
Compare
# Create and return log output content | ||
{ name: 'aws-sdk-ruby', | ||
function: "#{meth}(#{args_arr.join(',')})", # method name and arguments | ||
description: desc, |
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 understand function
will have just the API signature while args
will have the name:value
map.
Tested locally, overall looks good. As per #114 (comment), few comments:
|
IMO to ensure server quality all errors are |
One way to differentiate between error and alert IMO, is cases where there is data loss or data corruption or other severe impact happens (say you upload an object, and then download it and match against original file, and verification fails) can be marked as alert while rest are errors. |
63a98b4
to
2541fdb
Compare
Nitish,
|
- Also adds missing "duration" field and sets it up - Implemented comments and modified specs - Removes log output fields if nil
8b50977
to
c61bd9e
Compare
Tested locally, LGTM. Output:
|
running it. Seems to be ok. |
Issue #111