You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/01/28 15:45:23 UTC
[GitHub] [flink-statefun] igalshilman opened a new pull request #3:
[FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
igalshilman opened a new pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3
This PR addresses multiple build related issues.
1) Removes Lombok so that stateful functions can be built with Java11
2) Centralize exclusions in dependency management section
3) Add a comment for each exclustion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] tzulitai closed pull request #3:
[FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
Posted by GitBox <gi...@apache.org>.
tzulitai closed pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] igalshilman commented on a change in pull request
#3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
Posted by GitBox <gi...@apache.org>.
igalshilman commented on a change in pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3#discussion_r372270296
##########
File path: statefun-flink/pom.xml
##########
@@ -56,13 +56,68 @@ under the License.
<id>apache.snapshots</id>
<name>Apache Development Snapshot Repository</name>
<url>https://repository.apache.org/content/repositories/snapshots/</url>
- <releases><enabled>false</enabled></releases>
- <snapshots><enabled>true</enabled></snapshots>
+ <releases>
+ <enabled>false</enabled>
+ </releases>
+ <snapshots>
+ <enabled>true</enabled>
+ </snapshots>
</repository>
</repositories>
<dependencyManagement>
+ <!-- Flink -->
<dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-streaming-java_2.11</artifactId>
Review comment:
Thanks! your recommendation make sense, I will address that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] zentol commented on a change in pull request #3:
[FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3#discussion_r372240465
##########
File path: statefun-examples/statefun-ridesharing-example/statefun-ridesharing-example-simulator/pom.xml
##########
@@ -92,12 +102,6 @@ under the License.
<version>28.0-jre</version>
</dependency>
- <dependency>
- <groupId>org.projectlombok</groupId>
- <artifactId>lombok</artifactId>
Review comment:
can we separate this commit into 1 part that removes lombok, the other changing the parent pom? The commit message no longer covers the actual changes, and tbh it is a bit messy.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] zentol commented on a change in pull request #3:
[FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3#discussion_r372247146
##########
File path: statefun-flink/pom.xml
##########
@@ -56,13 +56,68 @@ under the License.
<id>apache.snapshots</id>
<name>Apache Development Snapshot Repository</name>
<url>https://repository.apache.org/content/repositories/snapshots/</url>
- <releases><enabled>false</enabled></releases>
- <snapshots><enabled>true</enabled></snapshots>
+ <releases>
+ <enabled>false</enabled>
+ </releases>
+ <snapshots>
+ <enabled>true</enabled>
+ </snapshots>
</repository>
</repositories>
<dependencyManagement>
+ <!-- Flink -->
Review comment:
you probably want to move this into the `<dependencies>` element
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] zentol commented on a change in pull request #3:
[FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3#discussion_r372246634
##########
File path: statefun-flink/pom.xml
##########
@@ -56,13 +56,68 @@ under the License.
<id>apache.snapshots</id>
<name>Apache Development Snapshot Repository</name>
<url>https://repository.apache.org/content/repositories/snapshots/</url>
- <releases><enabled>false</enabled></releases>
- <snapshots><enabled>true</enabled></snapshots>
+ <releases>
+ <enabled>false</enabled>
Review comment:
unrelated, belongs in separate commit
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] zentol commented on a change in pull request #3:
[FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3#discussion_r372245754
##########
File path: statefun-flink/pom.xml
##########
@@ -56,13 +56,68 @@ under the License.
<id>apache.snapshots</id>
<name>Apache Development Snapshot Repository</name>
<url>https://repository.apache.org/content/repositories/snapshots/</url>
- <releases><enabled>false</enabled></releases>
- <snapshots><enabled>true</enabled></snapshots>
+ <releases>
+ <enabled>false</enabled>
+ </releases>
+ <snapshots>
+ <enabled>true</enabled>
+ </snapshots>
</repository>
</repositories>
<dependencyManagement>
+ <!-- Flink -->
<dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-streaming-java_2.11</artifactId>
Review comment:
I would heavily recommend also defining the version in here; DependencyManagement entries without versions are not really intended and can break in weird ways when plugins try to resolve dependencies.
Depends a bit of course on which plugins you want to use in the future; we specifically ran into this when using the `japicmp` plugin which verifies API compatibility; which I suppose we may add in the future(?).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] igalshilman commented on a change in pull request
#3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
Posted by GitBox <gi...@apache.org>.
igalshilman commented on a change in pull request #3: [FLINK-15759][FLINK-15757][FLINK-15764] Adjust POM definitions
URL: https://github.com/apache/flink-statefun/pull/3#discussion_r372271064
##########
File path: statefun-examples/statefun-ridesharing-example/statefun-ridesharing-example-simulator/pom.xml
##########
@@ -92,12 +102,6 @@ under the License.
<version>28.0-jre</version>
</dependency>
- <dependency>
- <groupId>org.projectlombok</groupId>
- <artifactId>lombok</artifactId>
Review comment:
I agree that this is a bit messy, because removing Lombok also means replacing the classes with a full blown value classes.
I will separate these changes to two separate commits.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services