-
Notifications
You must be signed in to change notification settings - Fork 5
Partial refactoring of the s-pipes-core #432
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
base: main
Are you sure you want to change the base?
Conversation
@MSattrtand please rebase (not urgent) |
423895e
to
e25a4ac
Compare
|
||
@Parameter(iri = SML.replace) | ||
private boolean isReplace = false; | ||
private final boolean isReplace = false; |
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 does not make sense to make final because it is parameter of module
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.
reverted see my last commits and resolve after you read this.
private static String getValuesClauseValues(ResultSet resultSet, int rowsCount) { | ||
|
||
StringBuffer valuesBuffer = new StringBuffer(); | ||
StringBuilder valuesBuffer = new StringBuilder(); |
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.
What is the reason for this?
"\n\t - exception [3]: \"\n{}\n\"" | ||
, query, bindings, getStackTrace(ex)); | ||
log.error(""" | ||
Failed execution of query [1] for binding [2], due to exception [3]. \ |
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.
Failed execution of query [1] for binding [2], due to exception [3]. \ | |
Failed execution of query [1] for binding [2], due to exception [3]. |
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.
Not sure why you need this ... also check for other "" as well
a7c85bc
to
9ad2b05
Compare
public class SPINEnhancedNodeFactory extends Implementation { | ||
private final Node type; | ||
private Constructor enhNodeConstructor; | ||
private Constructor<?> enhNodeConstructor; |
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.
You removed the warning but did not really solve the issue that the warning was indicating .. see my last commits
|
||
} | ||
|
||
@Disabled |
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 go over all your changes within this PR and put back all @disabled tests ... I would like to address this outside of this issue ... in most cases i will want to implement those tests
Resolves partially #324