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

Fix incorrectly overridden project groups by JvmTestSuite. #4884

Merged
merged 2 commits into from
May 22, 2023

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented May 19, 2023

Motivation:

group was set in JvmTestSuite's configuration to change the test task's group, but Project.group was changed unintentionally.

armeria/core/build.gradle

Lines 178 to 180 in e15c7b7

testStreaming(JvmTestSuite) {
group = 'Verification'

The overridden group caused to publish Armeria artifacts to Verification org in Sonatype snapshot storage.
https://github.com/line/armeria/actions/runs/5010142242/jobs/8979748543#step:8:1273

Modifications:

  • Override the group of a test task in testTask.configure block.

Result:

Fixed unintentionally overridden Gradle project group information.

Motivation:

`group` was set in JvmTestSuite's configuration to change the test
task's group, but `Project.group` was changed unintentionally.
https://github.com/line/armeria/blob/e15c7b7809db6d77c6798c237707e942fceb51bb/core/build.gradle#L178-L180

The overriden group caused to publish a Armeria artifact to
`Verification` org in Sonatype snapshot storage.
https://github.com/line/armeria/actions/runs/5010142242/jobs/8979748543#step:8:1273

Modifications:

- Override the group of a test task in `testTask.configure` block.

Result:

Fixed unintentionally overridden Gradle project group information.
@ikhoon ikhoon added this to the 1.24.0 milestone May 19, 2023
Comment on lines 25 to 26

targets {
Copy link
Member

Choose a reason for hiding this comment

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

armeria/kotlin/build.gradle.kts:25:1 Needless blank line(s) (no-consecutive-blank-lines)

Suggested change
targets {
targets {

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix @ikhoon ! 👍 👍 👍

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

@ikhoon
Copy link
Contributor Author

ikhoon commented May 22, 2023

Thanks for the quick review. 🙇‍♂️

@ikhoon ikhoon merged commit f211673 into line:main May 22, 2023
@ikhoon ikhoon deleted the gradle-group-bug branch May 22, 2023 07:55
@ikhoon
Copy link
Contributor Author

ikhoon commented May 22, 2023

The publishing task successfully finished.
image

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.

4 participants