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

Build cleanup #19

Merged
merged 2 commits into from
Jul 8, 2022
Merged

Build cleanup #19

merged 2 commits into from
Jul 8, 2022

Conversation

sobychacko
Copy link
Collaborator

No description provided.

build.gradle Outdated
//if(gitPresent) {
// apply plugin: 'org.ajoberstar.grgit'
//}
if(gitPresent) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[TinyNit] Spacing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will address.

build.gradle Outdated

if (gitPresent) {
modifiedFiles =
files(grgit.status().unstaged.modified).filter{ f -> f.name.endsWith('.java') || f.name.endsWith('.kt') }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have any Kotlin files in here currently? If not, and we don't plan on adding anytime soon, I would be in favor of removing any Kotlin related pieces.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will remove.

Copy link
Collaborator

@onobc onobc left a comment

Choose a reason for hiding this comment

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

Nice cleanup @sobychacko . A few comments/questions but other than that, LGTM.

build.gradle Outdated
hibernateValidationVersion = '6.2.3.Final'
jacksonBomVersion = '2.13.2.20220328'
hibernateValidationVersion = '7.0.4.Final'
jacksonBomVersion = '2.13.3'
jaywayJsonPathVersion = '2.6.0'
junit4Version = '4.13.2'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we have junit4 in here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will remove.

hibernateValidationVersion = '6.2.3.Final'
jacksonBomVersion = '2.13.2.20220328'
hibernateValidationVersion = '7.0.4.Final'
jacksonBomVersion = '2.13.3'
jaywayJsonPathVersion = '2.6.0'
junit4Version = '4.13.2'
junitJupiterVersion = '5.8.2'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can instead pull in the junit bom under dependency mgmt which would get rid of some of these version specified I believe. Same as what spring-framework does.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, i just followed what Spring-Kafka does. We can do that. Feel free to make that change once this PR is merged.

}
}

task updateCopyrights {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice addition!

@@ -296,3 +352,14 @@ project ('spring-pulsar-sample-apps') {
}
}

sonarqube {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice addition! I saw this and the copyrights in spring-integration and was wanting to add it here as well. You beat me to it. :)

import org.springframework.pulsar.core.PulsarConsumerFactory;
import org.springframework.pulsar.core.PulsarProducerFactory;
import org.springframework.pulsar.core.PulsarTemplate;

/**
* {@link org.springframework.boot.autoconfigure.EnableAutoConfiguration Auto-configuration} for Apache Pulsar.
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Nit/Question] I wonder if it is sufficient to just link to org.springframework.boot.autoconfigure.Autoconfiguration now that its a 1st class thing in SB3?

Also, does this have to be fully specified like it is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, we can do it that way. Feel free to do that as part of the cleanup that you will do in the boot module.

@@ -162,12 +167,9 @@ subprojects { subproject ->
testImplementation "org.hamcrest:hamcrest-core:$hamcrestVersion"
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the actual dependencies, should we specify the versions in dependencyManagement section and then just specify group/artifiact in the actual dependencies?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

may not have to - i followed what SK does.

@sobychacko sobychacko merged commit 100e3f3 into spring-projects:main Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants