-
Notifications
You must be signed in to change notification settings - Fork 55
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
DR2 2047 allow modification of max chars per cell #532
base: master
Are you sure you want to change the base?
Conversation
def getColumnFromCsv(csvFile: TextFile, csvSchemaFile: TextFile, columnName: String): List[String] = Try { | ||
val validator = createValidator(true, Nil, false, false, false) | ||
val csv = validator.loadCsvFile(csvFile, csvSchemaFile) | ||
def getColumnFromCsv(csvFile: TextFile, csvSchemaFile: TextFile, columnName: String, maxCharsPerCell: Int): List[String] = Try { |
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.
Do we have to send the maxCharsPerCell to the validator as well as when calling a method on the validator?
If the validator knows about it, the method loadCsvFile
can rely on using this.maxCharsPerCell
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.
Ah, yes! Good point. I think I was just trying to get it to work initially, and didn't go back to clean up.
} | ||
|
||
private def validate(csvFile: String, csvEncoding: Charset, validateCsvEncoding: Boolean, csvSchemaFile: String, csvSchemaEncoding: Charset, validateCsvSchemaEncoding: Boolean, failFast: Boolean, pathSubstitutionsList: JList[Substitution], enforceCaseSensitivePathChecks: Boolean, trace: Boolean, progress: Option[SProgressCallback], skipFileChecks: Boolean): JList[FailMessage] = { | ||
private def validate(csvFile: String, csvEncoding: Charset, validateCsvEncoding: Boolean, csvSchemaFile: String, csvSchemaEncoding: Charset, validateCsvSchemaEncoding: Boolean, failFast: Boolean, pathSubstitutionsList: JList[Substitution], enforceCaseSensitivePathChecks: Boolean, trace: Boolean, progress: Option[SProgressCallback], skipFileChecks: Boolean, maxCharsPerCellLimit: Int): JList[FailMessage] = { |
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 we give the default value of 4096 here, so as to avoid the callers sending in 4096 as it looks like a magic number
toConsole("Error: Maximum number of errors to display should be more than 0") | ||
0 | ||
} | ||
val maxCharsPerCell = convertTextboxValueToInt(potentialMaxCharsPerCell, "errors to display", toConsole) |
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.
The error messages are swapped for textboxes
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.
Looks fine a couple of small changes.
And one thing to consider was about passing in "maxCharsPerCell" to validator class as well as methods on it and it seem to be in many places. I was thinking it should only be set on validator.
27dba83
to
96ff697
Compare
dfc925c
to
f5f44d9
Compare
f5f44d9
to
7ff37ba
Compare
No description provided.