-
Notifications
You must be signed in to change notification settings - Fork 55
migrated form gradle to maven #84
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
Conversation
f7dff0d to
eb2b526
Compare
| <modelVersion>4.0.0</modelVersion> | ||
| <groupId>com.github.Ericsson</groupId> | ||
| <artifactId>eiffel-remrem-semantics</artifactId> | ||
| <version>0.4.6</version> |
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.
comment about version uplift
pom.xml
Outdated
| <dependency> | ||
| <groupId>javax.inject</groupId> | ||
| <artifactId>javax.inject</artifactId> | ||
| <version>1</version> |
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 change this version
pom.xml
Outdated
| <dependency> | ||
| <groupId>com.github.fge</groupId> | ||
| <artifactId>json-schema-validator</artifactId> | ||
| <version>2.2.8</version> |
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.
use Globel version declaration
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 are using it in only one place, do we need it as global property?
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.
Yes, Good to have. Please follow same for all components.
pom.xml
Outdated
| <version>2.9.4</version> | ||
| <scope>compile</scope> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.glassfish.hk2.external/javax.inject --> |
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.
remove this comment
pom.xml
Outdated
| <dependency> | ||
| <groupId>com.github.fge</groupId> | ||
| <artifactId>json-schema-validator</artifactId> | ||
| <version>2.2.8</version> |
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.
Yes, Good to have. Please follow same for all components.
5e202dd to
5f16b13
Compare
|
|
||
| } | ||
|
|
||
| public Proxy getProxy() { |
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.
change it to private,
move the resource bundle handling to class level. and change this code accordingly.
Calling the getProxy() twice see if we can reduce the calling of this method
SantoshNC68
left a comment
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.
+1
pom.xml
Outdated
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <version>3.1</version> |
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.
check compatible with 3.6.1
raja-maragani
left a comment
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.
+1
made few changes in PrepareLocalEiffelSchemas class to clone the eiffel schema's repo behind the proxy also