-
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
GA release major changes. #21
Conversation
f22eecd
to
9a5b546
Compare
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.
Please format the code and add some java docs is possible.
@@ -376,35 +236,14 @@ public List<Map<String, Object>> fetchTableRecordsRetryableMode(String tableName | |||
*/ | |||
@Nullable | |||
public Schema fetchServiceNowTableSchema(String tableName, FailureCollector collector) { |
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.
seems this can be replaced by fetchTableSchema
.
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.
Renamed the method. 093d31e
|
||
return Double.parseDouble(String.valueOf(fieldValue)); | ||
try { | ||
return NumberFormat.getNumberInstance(java.util.Locale.US).parse(String.valueOf(fieldValue)).doubleValue(); |
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.
make sure that US
local parsing is expected to different regions or better to have a comment stating that US locale number format is hardcoded.
schema = restApi.fetchTableSchema(tableName); | ||
recordCount = restApi.getTableRecordCount(tableName); | ||
} catch (OAuthProblemException | OAuthSystemException e) { | ||
throw new RuntimeException(e); |
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.
Since only auth
exceptions are captured so please add some user friendly message for better understanding.
Ignore if you are already handling it somewhere.
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.
Added an exception message. 093d31e
Formatted the code and added Javadoc. 093d31e |
e464d65
to
bfb8fed
Compare
cbfb212
to
ad60671
Compare
9248f60
to
d281a19
Compare
143b520
to
728711b
Compare
728711b
to
74af16a
Compare
GA release major changes.