Skip to content

Dev springboot#25

Open
shub6691 wants to merge 4 commits intomasterfrom
dev_springboot
Open

Dev springboot#25
shub6691 wants to merge 4 commits intomasterfrom
dev_springboot

Conversation

@shub6691
Copy link
Copy Markdown
Contributor

@shub6691 shub6691 commented Sep 29, 2020

This project has been converted to spring boot from spring mvc.

Key factors to keep in mind while testing it..

  1. mvn clean install

  2. for generating the rpm files and testing it
    mvn clean install -Pbuild-app-rpm,build-xslt-rpm

  3. Also keep the usage schema version and snapshot version

  4. please also test for plugin of rpm build while testing in pom.xml

5 .Removed WEB-INF folder as not needed in spring boot project

@shub6691 shub6691 requested a review from AJStieren September 29, 2020 11:19
Copy link
Copy Markdown
Contributor

@AJStieren AJStieren left a comment

Choose a reason for hiding this comment

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

Some of the configs need to be migrated from the WEB-INF to keep filter functionality. Also advocating to move to log4j2 or at least update log4j.

Install was successful, tests passed, and builds produced the expected artifacts. Good job.

Comment thread pom.xml
Comment thread pom.xml Outdated
<groupId>org.springframework</groupId>
<artifactId>spring-webmvc</artifactId>
<version>${org.springframework.version}</version>
<groupId>log4j</groupId>
Copy link
Copy Markdown
Contributor

@AJStieren AJStieren Sep 29, 2020

Choose a reason for hiding this comment

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

Let's do log4j2. Actually we could probably add an slf4j overlay on the log4j2 as well for future proofing.

nvm, this is scope creep

Comment thread src/main/java/SpringBootMainApplication.java
@@ -1,67 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Dropping this means the much of this configuration needs to be migrated to the spring Boot. The filter mapping we have here is necessary. The logging hooks for Spring Boot should also be instantiated through log4j, though I'm advocating to move to log4j2 in line with Repose.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added lo4j2 config with the project.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, sorry... this is my bad, but I realized after the fact that moving from log4j to log4j2 is gross scope creep for the story. Move back to log4j and we'll do a logging enhancement story later.

Copy link
Copy Markdown
Contributor

@AJStieren AJStieren left a comment

Choose a reason for hiding this comment

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

Changes look good, rpm builds in a centos container, and tests are passing.

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