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

Add release workflow #13

Merged
merged 12 commits into from
Aug 28, 2023
Merged

Add release workflow #13

merged 12 commits into from
Aug 28, 2023

Conversation

supl
Copy link
Collaborator

@supl supl commented Aug 22, 2023

This PR adds a new workflow to release Java package and Docker image.

@supl supl self-assigned this Aug 22, 2023
Comment on lines +5 to +8
tags:
- v[0-9]+.[0-9]+.[0-9]+
branches:
- main
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This workflow will release tagged and SNAPSHOT versions

.github/workflows/release.yml Outdated Show resolved Hide resolved
build.gradle Outdated
@@ -8,7 +8,7 @@ subprojects {
}

group = "com.scalar-labs"
project.version = project.findProperty('projVersion') ?: '1.0.0-SNAPSHOT'
project.version = project.findProperty('projVersion') ?: '1.1.0-SNAPSHOT'
Copy link
Collaborator Author

@supl supl Aug 22, 2023

Choose a reason for hiding this comment

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

Since we are ready for 1.0.0, I rename the SNAPSHOT version to 1.1.0.
I would use the rolling release where features and bugfix are managed in the main branch only. There will be no branches to manage minor versions. This is the reason why the SNAPSHOT version in the main branch is not 2.0.0-SNAPSHOT.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should stick to the same model as the other products like ScalarDB and ScalarDL (i.e., the main branch points to the next major release); otherwise, it's too confusing for developers and users working on both projects.
Does it make things difficult?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@feeblefakie

I see.
For me, either {next major}.0.0-SNAPSHOT or 1.{next minor}.0 is reasonable as long as we clearly define the next is about major or minor.
Using {next major}.0.0-SNAPSHOT has a benefit of consistency so it's not difficult to me as well.

I changed the versioning in f8c8575

Comment on lines 10 to 11
name = 'Scalar Admin for Kubernetes'
description = 'This library provides a fault-tolerant manner to pause Scalar products in Kubernetes environments and ensure the data is transactionally consistent.'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please help revise the name and the description.

Copy link

@choplin choplin left a comment

Choose a reason for hiding this comment

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

Could you tell me why you want to publish this to the Maven repository as a library? As far as I understand, this is a CLI application. If so, it's not necessary to publish it to the Maven.

lib/build.gradle Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
cli/build.gradle Outdated Show resolved Hide resolved
gradle/wrapper/gradle-wrapper.properties Outdated Show resolved Hide resolved
@supl
Copy link
Collaborator Author

supl commented Aug 23, 2023

Could you tell me why you want to publish this to the Maven repository as a library? As far as I understand, this is a CLI application. If so, it's not necessary to publish it to the Maven.

@choplin
Thank you for your question.

In the original design, we made scalar-admin-k8s able to be integrated, for example, the Scalar Manager can use this library to pause the products in a Kubernetes cluster. The CLI is mainly for the place that could not use Java library, for example, the CronJob or the non-Java application.

@@ -83,7 +83,8 @@ done
# This is normally unused
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file is changed because my local Gradle was upgraded to 8.3 and I regenerate the Gradle wrapper to upgrade the wrapper from 7.6.2 to 8.1.1. Can ignore this diff.

@@ -2,5 +2,6 @@ distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file is changed because my local Gradle was upgraded to 8.3 and I regenerate the Gradle wrapper to upgrade the wrapper from 7.6.2 to 8.1.1. Can ignore this diff.

@choplin
Copy link

choplin commented Aug 23, 2023

@supl

In the original design, we made scalar-admin-k8s able to be integrated, for example, the Scalar Manager can use this library to pause the products in a Kubernetes cluster. The CLI is mainly for the place that could not use Java library, for example, the CronJob or the non-Java application.

Thank you for your explanation. Based on that, I'd recommend separating the project into sub-projects of cli and lib, and publishing only the lib project to the maven repo. IMO, it would not be good if the published library contained the main method. (You can do it later in another PR.)

@supl
Copy link
Collaborator Author

supl commented Aug 23, 2023

@supl

In the original design, we made scalar-admin-k8s able to be integrated, for example, the Scalar Manager can use this library to pause the products in a Kubernetes cluster. The CLI is mainly for the place that could not use Java library, for example, the CronJob or the non-Java application.

Thank you for your explanation. Based on that, I'd recommend separating the project into sub-projects of cli and lib, and publishing only the lib project to the maven repo. IMO, it would not be good if the published library contained the main method.

@choplin
I think it is exactly what I have done. (two sub projects and publish the Java package for only the lib part).

@supl supl requested a review from choplin August 23, 2023 06:54
@choplin
Copy link

choplin commented Aug 23, 2023

@supl

I think it is exactly what I have done. (two sub projects and publish the Java package for only the lib part).

Ah, sorry for that. I misunderstood. Thank you for pointing it out.

@supl supl requested a review from choplin August 23, 2023 08:15
Copy link

@choplin choplin left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@brfrn169 brfrn169 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

Left some suggestions. PTAL!

build.gradle Outdated
@@ -8,7 +8,7 @@ subprojects {
}

group = "com.scalar-labs"
project.version = project.findProperty('projVersion') ?: '1.0.0-SNAPSHOT'
project.version = project.findProperty('projVersion') ?: '1.1.0-SNAPSHOT'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should stick to the same model as the other products like ScalarDB and ScalarDL (i.e., the main branch points to the next major release); otherwise, it's too confusing for developers and users working on both projects.
Does it make things difficult?

lib/archive.gradle Outdated Show resolved Hide resolved
@supl supl requested a review from feeblefakie August 24, 2023 13:52
Copy link
Collaborator

@kota2and3kan kota2and3kan left a comment

Choose a reason for hiding this comment

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

Sorry for my late review...
Overall looks good to me, but I left one question.
Please take a look when you have time!

lib/archive.gradle Outdated Show resolved Hide resolved
Co-authored-by: kota2and3kan <[email protected]>
Copy link
Collaborator

@kota2and3kan kota2and3kan left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@feeblefakie feeblefakie merged commit be56cc0 into main Aug 28, 2023
@feeblefakie feeblefakie deleted the releasing branch August 28, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants