Skip to content
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

THRIFT-5564: setup java and kotlin lib building #2593

Merged
merged 31 commits into from May 3, 2022

Conversation

Jimexist
Copy link
Member

@Jimexist Jimexist commented Apr 28, 2022

setup java and kotlin lib building

this is a second step after:

  • Did you create an Apache Jira 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.

Copy link
Member

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I resolved the merge conflicts, but feel free to rebase yourself.

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@ctubbsii
Copy link
Member

I'm noticing the job name is quite long, which makes it hard to see the matrix params in the PR display. You can probably shorten it from "Build Thrift Compiler and Libraries" to just "Build" or similar.

You can also reduce the number of items in the matrix, and the output, by not making the Java distribution a matrix. It's probably sufficient to just use a single stable Java distribution. If temurin works, then just use that, but don't need to put it in the matrix.

You can also probably remove the Java 8 builds, since Java 11 should be using --release 8 when it builds anyway, if the intent is to support Java 8. It'd be more useful to include Java 17 (which is the current LTS), and Java 11 (former LTS, still popular).

@Jimexist
Copy link
Member Author

It'd be more useful to include Java 17 (which is the current LTS)

for now we can't because we are on gradle 6.9; we'll have to upgrade to gradle 7 first but that in turn replies on migrating from maven to maven-publish plugin

@Jens-G
Copy link
Member

Jens-G commented Apr 29, 2022

Just want to drop the usual reminder of maintaining the ReleaseManagement.md because we will need this.

@Jimexist
Copy link
Member Author

Just want to drop the usual reminder of maintaining the ReleaseManagement.md because we will need this.

thanks for the reminder @Jens-G, for now this and the currently open pull requests only adds CI part, not the CD part, also the travis CI is intact - so I have no changes to the ReleaseManagement.md just yet. When I do later make the final switch, I'll post to dev@thrift and update accordingly. For now, there's no change needed.

.github/workflows/build.yml Show resolved Hide resolved
@ctubbsii ctubbsii merged commit c77d91a into apache:master May 3, 2022
@Jimexist Jimexist deleted the setup-github-action-java branch May 3, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants