You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2022/04/12 06:22:27 UTC
[GitHub] [thrift] Jimexist opened a new pull request, #2561: java library to use newer gradle API
Jimexist opened a new pull request, #2561:
URL: https://github.com/apache/thrift/pull/2561
<!-- Explain the changes in the pull request below: -->
java library to use newer gradle API
<!-- We recommend you review the checklist/tips before submitting a pull request. -->
- [ ] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? (not required for trivial changes)
- [ ] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [ ] Did you squash your changes to a single commit? (not required, but preferred)
- [ ] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
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.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1101557163
> I agree with that GitHub Actions more user friendly and efficient. If that's a direction that the community wishes to move towards I'd be able to help.
That would be great. I think they could replace AppVeyor as well.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1101904662
> > I agree with that GitHub Actions more user friendly and efficient. If that's a direction that the community wishes to move towards I'd be able to help.
>
> That would be great. I think they could replace AppVeyor as well.
Let me create https://issues.apache.org/jira/browse/THRIFT-5564 and wait for some more feedback if any.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1120097491
> Looks fine to me. Will wait for CI. In the meantime, @Jimexist , please make sure that there wasn't any errors introduced while resolving merge conflicts, and that this is still ready to go.
thanks! confirmed on my side that this is 👍
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2561:
URL: https://github.com/apache/thrift/pull/2561#discussion_r849019086
##########
lib/java/gradle/environment.gradle:
##########
@@ -64,14 +64,14 @@ repositories {
}
dependencies {
- compile "org.slf4j:slf4j-api:${slf4jVersion}"
- compile "org.apache.httpcomponents:httpclient:${httpclientVersion}"
- compile "org.apache.httpcomponents:httpcore:${httpcoreVersion}"
- compile "javax.servlet:javax.servlet-api:${servletVersion}"
- compile "javax.annotation:javax.annotation-api:${javaxAnnotationVersion}"
- compile "org.apache.commons:commons-lang3:3.12.0"
+ implementation "org.slf4j:slf4j-api:${slf4jVersion}"
+ implementation "org.apache.httpcomponents:httpclient:${httpclientVersion}"
+ implementation "org.apache.httpcomponents:httpcore:${httpcoreVersion}"
+ implementation "javax.servlet:javax.servlet-api:${servletVersion}"
+ implementation "javax.annotation:javax.annotation-api:${javaxAnnotationVersion}"
+ implementation "org.apache.commons:commons-lang3:3.12.0"
- testCompile "junit:junit:${junitVersion}"
- testCompile "org.mockito:mockito-all:${mockitoVersion}"
- testRuntime "org.slf4j:slf4j-log4j12:${slf4jVersion}"
+ testImplementation "junit:junit:${junitVersion}"
+ testImplementation "org.mockito:mockito-all:${mockitoVersion}"
+ testRuntimeOnly "org.slf4j:slf4j-log4j12:${slf4jVersion}"
Review Comment:
```suggestion
testRuntime"org.slf4j:slf4j-log4j12:${slf4jVersion}"
```
##########
lib/java/gradle/functionalTests.gradle:
##########
@@ -59,18 +59,18 @@ shadowJar {
// make sure the runners are created when this runs
dependsOn 'generateRunnerScriptForClient', 'generateRunnerScriptForServer', 'generateRunnerScriptForNonblockingServer', 'generateRunnerScriptForTServletServer'
- baseName = 'functionalTest'
- destinationDir = file("$buildDir/functionalTestJar")
+ archiveBaseName = 'functionalTest'
+ destinationDirectory = file("$buildDir/functionalTestJar")
classifier = null
// We do not need a version number for this internal jar
- version = null
+ archiveVersion = null
// Bundle the complete set of unit test classes including generated code
// and the runtime dependencies in one JAR to expedite execution.
from sourceSets.test.output
from sourceSets.crossTest.output
- configurations = [project.configurations.testRuntime]
+ configurations = [project.configurations.testRuntimeOnly]
Review Comment:
```suggestion
configurations = [project.configurations.testRuntime]
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1120237652
> > However, it still shows a warning that `Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.`
>
> yes because we still have one thing left: migrate `maven` plugin to `maven-publish` plugin. that's a bigger task to handle so i'll do it later, along with upgrading to gradle 7.x
I've put up:
- #2601 to migrate to gradle 7.4+ which is the latest version
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1101130882
> > i'm not sure i can debug the CI failures - seems an issue with travis itself as the failures do not have logs and they occur randomly (same situation with other pull requests)
>
> I don't think those failures are related to this change, but I can't be sure. The individual tasks aren't well named and I'm not sure what they're doing.
>
> Also, somebody familiar with the Travis CI configuration might want to investigate moving over to GitHub Actions, since 1) it's built-in to GitHub and possibly a bit more reliable/less dependencies on another party, and 2) I don't trust Travis CI's new permissions model that seems to require granting it oauth permissions for private repos in order to log in to Travis-CI.com to manage builds, even if you only want to use it with public repos. I wonder if that was partially the reason for [this recent security incident involving Travis CI app being targeted and used as a mechanism to steal private repo contents](https://github.blog/2022-04-15-security-alert-stolen-oauth-user-tokens/)
>
> I know one of the main limits of GitHub Actions is the inability to use ARM or other non-x86 architectures when building. As a Java developer I don't generally care about that, but I'm guessing the Thrift project probably cares.
I agree with that GitHub Actions more user friendly and efficient. If that's a direction that the community wishes to move towards I'd be able to help.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii merged pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
ctubbsii merged PR #2561:
URL: https://github.com/apache/thrift/pull/2561
--
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.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1113881286
FYI i also had to update tutorial/js and tutorial/java since their ant build refers to the legacy java libs
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1119275950
> However, it still shows a warning that `Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.`
yes because we still have one thing left: migrate `maven` plugin to `maven-publish` plugin. that's a bigger task to handle so i'll do it later, along with upgrading to gradle 7.x
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2561:
URL: https://github.com/apache/thrift/pull/2561#discussion_r849019165
##########
lib/java/gradle/publishing.gradle:
##########
@@ -46,7 +46,7 @@ task copyDependencies(type: Copy, group: 'Build') {
project.assemble.dependsOn it
destinationDir = file("$buildDir/deps")
- from configurations.testRuntime
+ from configurations.testRuntimeOnly
Review Comment:
```suggestion
from configurations.testRuntime
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1119270511
@Jimexist I merged master into this one to bring it up to date. I ran a `gradle build` and it seems to work fine with Gradle 6.9.2. However, it still shows a warning that `Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1101021601
i'm not sure i can debug the CI failures - seems an issue with travis itself as the failures do not have logs and they occur randomly (same situation with other pull requests)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2561: THRIFT-5553: java library to use newer gradle API
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2561:
URL: https://github.com/apache/thrift/pull/2561#issuecomment-1101070469
> i'm not sure i can debug the CI failures - seems an issue with travis itself as the failures do not have logs and they occur randomly (same situation with other pull requests)
I don't think those failures are related to this change, but I can't be sure. The individual tasks aren't well named and I'm not sure what they're doing.
Also, somebody familiar with the Travis CI configuration might want to investigate moving over to GitHub Actions, since 1) it's built-in to GitHub and possibly a bit more reliable/less dependencies on another party, and 2) I don't trust Travis CI's new permissions model that seems to require granting it oauth permissions for private repos in order to log in to Travis-CI.com to manage builds, even if you only want to use it with public repos. I wonder if that was partially the reason for [this recent security incident involving Travis CI app being targeted and used as a mechanism to steal private repo contents](https://github.blog/2022-04-15-security-alert-stolen-oauth-user-tokens/)
I know one of the main limits of GitHub Actions is the inability to use ARM or other non-x86 architectures when building. As a Java developer I don't generally care about that, but I'm guessing the Thrift project probably cares.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org