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/21 03:07:53 UTC

[GitHub] [thrift] Jimexist opened a new pull request, #2583: update java lib document about gradle usage

Jimexist opened a new pull request, #2583:
URL: https://github.com/apache/thrift/pull/2583

   <!-- Explain the changes in the pull request below: -->
     update java lib document about gradle usage
   
   <!-- 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] Jimexist commented on a diff in pull request #2583: THRIFT-5570: update java lib document about gradle usage

Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2583:
URL: https://github.com/apache/thrift/pull/2583#discussion_r866432420


##########
lib/java/.gitignore:
##########
@@ -0,0 +1,53 @@
+# Created by https://www.toptal.com/developers/gitignore/api/java,gradle
+# Edit at https://www.toptal.com/developers/gitignore?templates=java,gradle
+

Review Comment:
   fixed in https://github.com/apache/thrift/pull/2583/commits/85c18bd6b15d45d34098e902c4337cdb7401ce2b



-- 
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 #2583: THRIFT-5570: update java lib document about gradle usage

Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2583:
URL: https://github.com/apache/thrift/pull/2583#issuecomment-1106029961

   had to rebase to pickup fix from #2585 


-- 
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 a diff in pull request #2583: THRIFT-5570: update java lib document about gradle usage

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2583:
URL: https://github.com/apache/thrift/pull/2583#discussion_r855420366


##########
doc/install/debian.md:
##########
@@ -18,7 +18,7 @@ Debian 7/Ubuntu 12 users need to manually install a more recent version of autom
 If you would like to build Apache Thrift libraries for other programming languages you may need to install additional packages. The following languages require the specified additional packages:
 
  * Java
-	* packages: gradle 
+	* packages: gradle (version 6.9.2)

Review Comment:
   I don't know that the Gradle version is a fixed requirement. Can clarify that this was the version tested:
   
   ```suggestion
   	* packages: gradle (version 6.9.2 is confirmed to work)
   ```



##########
lib/java/.gitignore:
##########
@@ -0,0 +1,53 @@
+# Created by https://www.toptal.com/developers/gitignore/api/java,gradle
+# Edit at https://www.toptal.com/developers/gitignore?templates=java,gradle
+
+### Java ###
+# Compiled class file
+*.class
+
+# Log file
+*.log
+
+# BlueJ files
+*.ctxt
+
+# Mobile Tools for Java (J2ME)
+.mtj.tmp/
+
+# Package Files #
+*.jar
+*.war
+*.nar
+*.ear
+*.zip
+*.tar.gz
+*.rar
+
+# virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml
+hs_err_pid*
+replay_pid*
+
+### Gradle ###
+.gradle
+**/build/
+!src/**/build/
+
+# Ignore Gradle GUI config
+gradle-app.setting
+
+# Cache of project
+.gradletasknamecache
+
+# Eclipse Gradle plugin generated files
+# Eclipse Core
+.project
+# JDT-specific (Eclipse Java Development Tools)
+.classpath

Review Comment:
   Eclipse has a .settings/ directory also:
   
   ```suggestion
   .classpath
   .settings/
   ```



##########
lib/java/README.md:
##########
@@ -42,11 +42,41 @@ The Thrift Java source is not build using the GNU tools, but rather uses
 the Gradle build system, which tends to be predominant amongst Java
 developers.
 
+Currently we use gradle 6.9.2 to build the Thrift Java source. The usual way to setup gradle
+project is to include the gradle-wrapper.jar in the project and then run the gradle wrapper to
+bootstrap setting up gradle binaries. However to avoid putting binary files into the source tree we
+have ignored the gradle wrapper files. You are expected to install it manually, as described in
+the [gradle documentation](https://docs.gradle.org/current/userguide/installation.html), or
+following this step (which is also done in the travis CI docker images):
+
+```bash
+export GRADLE_VERSION="6.9.2"
+# install dependencies
+apt-get install -y --no-install-recommends openjdk-11-jdk-headless wget unzip
+# download gradle distribution
+wget https://services.gradle.org/distributions/gradle-$GRADLE_VERSION-bin.zip -q -O /tmp/gradle-$GRADLE_VERSION-bin.zip
+# check binary integrity
+echo "8b356fd8702d5ffa2e066ed0be45a023a779bba4dd1a68fd11bc2a6bdc981e8f  /tmp/gradle-$GRADLE_VERSION-bin.zip" | sha256sum -c -
+# unzip and install
+unzip -d /tmp /tmp/gradle-$GRADLE_VERSION-bin.zip
+mv /tmp/gradle-$GRADLE_VERSION /usr/local/gradle
+ln -s /usr/local/gradle/bin/gradle /usr/local/bin
+```
+
+After the above step, `gradle` binary will be available in `/usr/local/bin/`. You can further choose
+to locally create the gradle wrapper (even if they are ignored) using:
+
+```bash
+gradle wrapper --gradle-version $GRADLE_VERSION
+```
+
 To compile the Java Thrift libraries, simply do the following:
 
-    gradle
+```bash
+gradle
+```
 
-Yep, that's easy. Look for libthrift-<version>.jar in the build/libs directory.
+Yep, that's easy. Look for `libthrift-<version>.jar` in the build/libs directory.

Review Comment:
   We probably shouldn't say it's easy... that's subjective. Users may not agree, and might interpret this as being arrogant.
   ```suggestion
   Look for `libthrift-<version>.jar` in the build/libs directory.
   ```



##########
lib/java/README.md:
##########
@@ -42,11 +42,41 @@ The Thrift Java source is not build using the GNU tools, but rather uses
 the Gradle build system, which tends to be predominant amongst Java
 developers.
 
+Currently we use gradle 6.9.2 to build the Thrift Java source. The usual way to setup gradle
+project is to include the gradle-wrapper.jar in the project and then run the gradle wrapper to
+bootstrap setting up gradle binaries. However to avoid putting binary files into the source tree we
+have ignored the gradle wrapper files. You are expected to install it manually, as described in
+the [gradle documentation](https://docs.gradle.org/current/userguide/installation.html), or
+following this step (which is also done in the travis CI docker images):
+
+```bash
+export GRADLE_VERSION="6.9.2"
+# install dependencies
+apt-get install -y --no-install-recommends openjdk-11-jdk-headless wget unzip
+# download gradle distribution
+wget https://services.gradle.org/distributions/gradle-$GRADLE_VERSION-bin.zip -q -O /tmp/gradle-$GRADLE_VERSION-bin.zip
+# check binary integrity
+echo "8b356fd8702d5ffa2e066ed0be45a023a779bba4dd1a68fd11bc2a6bdc981e8f  /tmp/gradle-$GRADLE_VERSION-bin.zip" | sha256sum -c -
+# unzip and install
+unzip -d /tmp /tmp/gradle-$GRADLE_VERSION-bin.zip
+mv /tmp/gradle-$GRADLE_VERSION /usr/local/gradle
+ln -s /usr/local/gradle/bin/gradle /usr/local/bin
+```
+
+After the above step, `gradle` binary will be available in `/usr/local/bin/`. You can further choose
+to locally create the gradle wrapper (even if they are ignored) using:

Review Comment:
   ```suggestion
   After the above step, the `gradle` binary will be available in `/usr/local/bin/`. You may then choose
   to locally create the Gradle Wrapper, if you wish to run commands with `gradlew` instead of `gradle`, using:
   ```



##########
lib/java/README.md:
##########
@@ -42,11 +42,41 @@ The Thrift Java source is not build using the GNU tools, but rather uses
 the Gradle build system, which tends to be predominant amongst Java
 developers.
 
+Currently we use gradle 6.9.2 to build the Thrift Java source. The usual way to setup gradle
+project is to include the gradle-wrapper.jar in the project and then run the gradle wrapper to
+bootstrap setting up gradle binaries. However to avoid putting binary files into the source tree we
+have ignored the gradle wrapper files. You are expected to install it manually, as described in
+the [gradle documentation](https://docs.gradle.org/current/userguide/installation.html), or
+following this step (which is also done in the travis CI docker images):

Review Comment:
   Minor punctuation, capitalization, and phrasing improvements:
   
   ```suggestion
   Currently, we use Gradle 6.9.2 to build the Thrift Java source. The usual way to setup Gradle
   project is to include the gradle-wrapper.jar in the project and then run the Gradle Wrapper to
   bootstrap setting up Gradle binaries. However, to avoid putting binary files into the source tree, we
   have excluded the Gradle Wrapper files. You are expected to install Gradle manually, as described in
   the [Gradle documentation](https://docs.gradle.org/current/userguide/installation.html), or
   following the following steps (which is also done in the Travis CI Docker images):
   ```



-- 
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 #2583: THRIFT-5570: update java lib document about gradle usage

Posted by GitBox <gi...@apache.org>.
ctubbsii merged PR #2583:
URL: https://github.com/apache/thrift/pull/2583


-- 
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 a diff in pull request #2583: THRIFT-5570: update java lib document about gradle usage

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2583:
URL: https://github.com/apache/thrift/pull/2583#discussion_r866294649


##########
lib/java/.gitignore:
##########
@@ -0,0 +1,53 @@
+# Created by https://www.toptal.com/developers/gitignore/api/java,gradle
+# Edit at https://www.toptal.com/developers/gitignore?templates=java,gradle
+

Review Comment:
   This change adds a new .gitignore file in the lib/java directory. This is consistent with the recently added one for lib/kotlin. However, they do differ in some ways. And, a lot of this is redundant with the top-level .gitignore file.
   
   I think it'd be better to remove both of these .gitignore files and add the relevant patterns into the .gitignore file at the root of the repository, rather than add this here.



-- 
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