-
Notifications
You must be signed in to change notification settings - Fork 67
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
Build cleanup #19
Conversation
build.gradle
Outdated
//if(gitPresent) { | ||
// apply plugin: 'org.ajoberstar.grgit' | ||
//} | ||
if(gitPresent) { |
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.
[TinyNit] Spacing
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.
will address.
build.gradle
Outdated
|
||
if (gitPresent) { | ||
modifiedFiles = | ||
files(grgit.status().unstaged.modified).filter{ f -> f.name.endsWith('.java') || f.name.endsWith('.kt') } |
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.
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.
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.
will remove.
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.
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' |
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.
Why do we have junit4 in here?
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.
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' |
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 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.
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.
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 { |
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.
Nice addition!
@@ -296,3 +352,14 @@ project ('spring-pulsar-sample-apps') { | |||
} | |||
} | |||
|
|||
sonarqube { |
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.
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. |
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.
[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?
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.
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" |
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.
For the actual dependencies, should we specify the versions in dependencyManagement section and then just specify group/artifiact in the actual dependencies?
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.
may not have to - i followed what SK does.
No description provided.