-
Notifications
You must be signed in to change notification settings - Fork 37
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
Use maven-publish #258
Use maven-publish #258
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Thank you!
Left some questions and minor comments.
from javadoc | ||
} | ||
|
||
task testJar(type: Jar) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just curious. Any reason you decided not to publish test
and testSources
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including test
and testSources
jars to a release artifact seems to be a bit tricky in the maven-publish
plugin. And I rethought why we needed to publish them, and I thought we didn't need to do that, at least for now.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine not to have them since we don't really use them.
Thank you!
@@ -1,4 +1,4 @@ | |||
(defproject scalar-schema "3.0.0" | |||
(defproject scalar-schema "4.0.0-SNAPSHOT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for 4.0.0-SNAPSHOT
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because the master
branch is for the next release, 4.0.0
.
Similarly, I changed the version to 3.0.2-SNAPSHOT
in the 3.0
branch, and I'm going to change the versions to 3.2.0-SNAPSHOT
and 3.1.1-SNAPSHOT
in the 3
and 3.1
branches, respectively.
I think the version should be the next release version in the branch + -SNAPSHOT
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't know that's the way it is. Thank you!
testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.9.1' | ||
testImplementation group: 'org.mockito', name: 'mockito-core', version: '2.16.0' | ||
testImplementation group: 'org.mockito', name: 'mockito-inline', version: '2.16.0' | ||
integrationTestScalarDbServerImplementation project(':core').sourceSets.integrationTest.output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question for my future update for the ledger project.
Any reason for using integrationTestScalarDbServerImplementation
instead of testImplementation
?
Co-authored-by: Hiroyuki Yamada <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@feeblefakie Thank you for taking a look at this!
I just left some replies on your review comments. Please check them when you have time! Thanks.
from javadoc | ||
} | ||
|
||
task testJar(type: Jar) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including test
and testSources
jars to a release artifact seems to be a bit tricky in the maven-publish
plugin. And I rethought why we needed to publish them, and I thought we didn't need to do that, at least for now.
What do you think?
@@ -1,4 +1,4 @@ | |||
(defproject scalar-schema "3.0.0" | |||
(defproject scalar-schema "4.0.0-SNAPSHOT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because the master
branch is for the next release, 4.0.0
.
Similarly, I changed the version to 3.0.2-SNAPSHOT
in the 3.0
branch, and I'm going to change the versions to 3.2.0-SNAPSHOT
and 3.1.1-SNAPSHOT
in the 3
and 3.1
branches, respectively.
I think the version should be the next release version in the branch + -SNAPSHOT
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
I realized that the
maven
plugin is removed from gradle 7.0, and we need to use themaven-publish
plugin instead. This PR will change to use themaven-publish
plugin. And after this fix, tests jar and test-sources jar won't be published.Please take a look!