-
Notifications
You must be signed in to change notification settings - Fork 6
Springboot Version update #16
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
Conversation
build.gradle
Outdated
| @@ -1,7 +1,7 @@ | |||
| plugins | |||
| /*plugins | |||
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 whole comment out section should be removed.
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,I have removed this whole comment out section.
build.gradle
Outdated
| //*/ | ||
| plugins | ||
| { | ||
| id 'org.springframework.boot' version '3.3.5' |
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.
A higher stable version is available, we should move to that.
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,I have updated the version.
build.gradle
Outdated
| sourceCompatibility = JavaVersion.VERSION_17 | ||
| targetCompatibility = JavaVersion.VERSION_17 | ||
| } | ||
| //java |
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.
Clean-up and remove all old comments please.
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.
Done. I’ve cleaned up and removed all old comments.
build.gradle
Outdated
| // url "https://public.dhe.ibm.com/ibmdl/export/pub/software/websphere/maven/repository/" | ||
| //} | ||
| } | ||
| 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.
Remove commented out libraries, and add good comments to each library that is used.
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,I’ve removed the commented-out libraries and added appropriate comments for each library that is used.
build.gradle
Outdated
| // Not needed | ||
| //implementation("org.springframework.boot:spring-boot-starter-actuator") | ||
| // Use correct BOM version for CICS TS | ||
| compileOnly(enforcedPlatform("com.ibm.cics:com.ibm.cics.ts.bom:6.2-20250528120756-PH65227")) |
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.
We should level-set on CICS TS V6.1 as our min CICS release - add a comment to suggest users can move to a higher level of BOM to match their target CICS region.
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.
Done,I've updated the cics bom version to 6.1 and added a comment for users for the same as well.
build.gradle
Outdated
| //{ | ||
| // enabled = false // disable plain WAR, use bootWar instead | ||
| //} | ||
| publishing { |
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 not clear on why this section has changed from the original?
pom.xml
Outdated
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-parent</artifactId> | ||
| <version>2.7.0</version> | ||
| <version>3.3.5</version> |
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.
Version need updating.
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,done
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-web</artifactId> | ||
| </dependency> | ||
| <dependency> |
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 dependency has been separated from it's comment above and needs to be matched back up.
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.
Thanks for pointing this out — I’ve moved the comment so it now correctly matches the dependency.
| { | ||
| SpringApplication.run(Application.class, args); | ||
| } | ||
| public class Application { |
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.
brackets should be CICS style, and not end-of-line.
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,done
IvanHargreaves
left a comment
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.
Hi Manvi. I've made various comments on the PR, hopefully you can work your way through them asking questions and learning about the types of rigour and best-practice we apply to the samples.
|
Hey Ivan, I’ve worked through your comments and tried my best to address and fix them. Thankyou, and please let me know if any further corrections are needed. |
IvanHargreaves
left a comment
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.
A few more things to tweak.
.classpath
Outdated
| </classpathentry> | ||
| <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8/"/> | ||
| <classpathentry kind="output" path="bin/default"/> | ||
| <classpathentry excluding="**" kind="src" output="target/classes" path="src/main/resources"> |
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.
A lot of this added configuration is Maven-centric, the intention of the samples was to avoid making it specific to Maven or Gradle, so I think we have to remove a lot of this. The instructions in the README cover how to make it Maven, or Gradle specific after it's cloned to the users workspace.
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,I have changed the classpath,please have a look into it.
build.gradle
Outdated
| // JCICS API (version inherited from BOM) | ||
| compileOnly("com.ibm.cics:com.ibm.cics.server") | ||
| // Jakarta EE servlet API | ||
| compileOnly("jakarta.servlet:jakarta.servlet-api:6.0.0") |
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 forget now, but did we need to add this servlet dependency? There wasn't there one before this change so I'm just double checking.
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.
Yeah,I did my mistake,I just checked that its not there in pom.xml.I tested the sample again by building and deploying it, and verified that the endpoints are working fine without it.
build.gradle
Outdated
| group = 'com.ibm.cicsdev.springboot' | ||
| archivesBaseName='cics-java-liberty-springboot-jcics' | ||
| version = '0.1.0' | ||
| // No longer relevant |
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.
Please delete the code that's no longer relevant, rather than comment it out.
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,done
pom.xml
Outdated
| </properties> | ||
|
|
||
| <!-- CICS TS V5.5 BOM (as of May 2020) --> | ||
| <!-- CICS TS V6.1 BOM (as of May 2020) --> |
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 2020 is not the correct date for the V6.1 BOM. The version string suggests Sept 2024 is the matching date.
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.
sorry,missed that,edited.
No description provided.