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

Dev springboot #25

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Dev springboot #25

wants to merge 4 commits into from

Conversation

shub6691
Copy link
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
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.

pom.xml Show resolved Hide resolved
pom.xml Outdated
<groupId>org.springframework</groupId>
<artifactId>spring-webmvc</artifactId>
<version>${org.springframework.version}</version>
<groupId>log4j</groupId>
Copy link
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

src/main/java/SpringBootMainApplication.java Show resolved Hide resolved
@@ -1,67 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
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
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
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
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