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

Add exploded loose app support #1289

Merged
merged 1 commit into from
Dec 7, 2021
Merged

Conversation

awisniew90
Copy link
Contributor

Signed-off-by: Adam Wisniewski [email protected]

Add support for "exploded" loose war applications.

This will:

  1. Auto-detect if the exploded function is needed (checking if WAR overlays and/or resource filtering is configured)
  2. Tailor the loose app xml to use the "filtered" web resources location
  3. Run the war plugin's "exploded" goal on changes
  4. Dynamically switch between "exploded" and "non-exploded" (default) mode in dev mode as filtering/overlays are added/removed.

@awisniew90
Copy link
Contributor Author

Note: This PR does not yet add this support into the current multi-module support. To avoid having this PR get any bigger, I'm going to add that in after this.

@awisniew90
Copy link
Contributor Author

Technical Debt

  1. Issue warning on "self overlay"

@awisniew90
Copy link
Contributor Author

Tests are failing due to the requirement on ci.common changes here: OpenLiberty/ci.common#309. Once that PR is merged, the test failures should be resolved.

Copy link
Member

@scottkurz scottkurz left a comment

Choose a reason for hiding this comment

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

I left some comments. Spent a good amt. of time just remembering some of the code.

I wonder if we should do a walkthru review to confirm the updates in slide 17?

Copy link
Member

@scottkurz scottkurz left a comment

Choose a reason for hiding this comment

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

More comments

Copy link
Contributor

@ericglau ericglau left a comment

Choose a reason for hiding this comment

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

I haven't looked at the other parts of this PR in detail, but the DevMojo changes looks fine to me.

@awisniew90 awisniew90 force-pushed the exploded_war branch 2 times, most recently from 67fa5ad to da1c447 Compare November 22, 2021 20:37
@awisniew90 awisniew90 force-pushed the exploded_war branch 2 times, most recently from 71561e3 to d692088 Compare November 30, 2021 19:43
Copy link
Member

@cherylking cherylking left a comment

Choose a reason for hiding this comment

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

See my comments / questions. Overall, looks good. Just need some clarifications.

@awisniew90 awisniew90 force-pushed the exploded_war branch 6 times, most recently from d0bea75 to 170e9d5 Compare December 3, 2021 15:38
@awisniew90 awisniew90 force-pushed the exploded_war branch 2 times, most recently from f34bbbf to fe67e2f Compare December 6, 2021 22:11
Signed-off-by: Adam Wisniewski <[email protected]>
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.

None yet

4 participants