-
Notifications
You must be signed in to change notification settings - Fork 19
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
Deterministic Code Generation #65
Conversation
Wow! This is quite comprehensive. We will have a look as soon as we can! |
@SergejT34 could you contact me on [email protected]? I would like to set up a meeting to discuss these changes |
<includes> | ||
<include>sdk-generate-entity-models-maven-plugin/src/**/java/**/*.java</include> | ||
</includes> | ||
<eclipse> |
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 like the idea of using a code formatter, however will this work (integrate) with IntelliJ, we use that internally?
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.
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.
we also use intellij at emundo 😉 and all our projects has a build step in our ci to enforce correct formatting.
https://github.com/diffplug/spotless/tree/master/plugin-gradle
https://github.com/diffplug/spotless/tree/master/plugin-maven
@@ -188,6 +189,27 @@ | |||
<!-- mvn com.mycila:license-maven-plugin:format (add headers if missing) --> | |||
<!-- mvn com.mycila:license-maven-plugin:remove (remove existing header) --> |
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.
these for comment lines need to be next to the relevant maven plugin
* @param doNotValidateCertificate - Disables validating server SSL certificates | ||
* @return a new Octane instance which has the set context and is correctly authenticated | ||
*/ | ||
public Octane build(final boolean doNotValidateCertificate) { |
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.
IMO this flag should be a step in the build process, with a default value. Going by our current "convention" I guess it would be a method called SkipSslCertificateValidation(boolean skipCertificateValidation).
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 need to think about this one
this(urlDomain, false); | ||
} | ||
|
||
public GoogleHttpClient(final String urlDomain, final boolean doNotValidateCertificate) { |
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.
Good to have :) ! Sad that it's needed.
8d246e0
to
58f4466
Compare
…es statement introduced in java 1.7
…t enums by name in ASC order
…r SSL certificates
improved List template
Improved Phase template
58f4466
to
72e6bf0
Compare
We decided to fix some of these issues in different ways. You can see the updates. If there are still issues that you want to fix open a new PR (but make them smaller and more self-contained! :) ) |
Problem:
There are some Octane Enterprise installations behind a firewall, so code generation could not be done on each build.
Solution:
Change the code generator to generate reproducible java code:
=> run generator twice, and the generated code will remain the same
Improvements:
Lists
,Phases
,EntityModels
&Entities
@Generated
Annotation for ignoring by code coverage analyzersdoNotValidateCertificate
to ignore self signed SSL certificates (default is false for legacy).