Skip to content

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 = '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.

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!

}
}

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.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.

testImplementation("org.awaitility:awaitility:$awaitilityVersion") {
exclude group: 'org.hamcrest'
}
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