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

Smaller cleanups #25172

Merged
merged 19 commits into from
Oct 8, 2024
Merged

Smaller cleanups #25172

merged 19 commits into from
Oct 8, 2024

Conversation

dmatej
Copy link
Contributor

@dmatej dmatej commented Oct 7, 2024

This PR contains mostly updated syntax (like using enhanced loops), usual conventions (like fields first, then methods), but contains also several important changes in managing exceptions, usually stop just ignoring them, but throw them.

I have yet one large PR to push where I touched classloaders and logging to resolve #25133 and these issues blocked me for a while. I believe they caused headaches to many GlassFish users over the time, for example when I called asadmin restart-domain, domain stopped but did not start with no log and no output. Now it will print why that happened.

Note: I did this work in one branch first with many uncommitted changes, then I tried to separate them and rebuilt GlassFish with every few changes cherrypicked to this one. The problem with classloaders is that when you touch them, the next step is quite long until you are on the safe ground again. So I created this PR to make your work bit easier - maybe I will create yet few other and shrink the most important part as much as possible. Please, have patience with me.

Signed-off-by: David Matějček <[email protected]>
- don't swallow exceptions
- don't catch Error instances

Signed-off-by: David Matějček <[email protected]>
Signed-off-by: David Matějček <[email protected]>
Signed-off-by: David Matějček <[email protected]>
- another file is in the same package in resources
- JSPCompiler already uses LogFacade

Signed-off-by: David Matějček <[email protected]>
- Error messages now provide more info about possible causes
- If restart fails, throw Error as domain is simply dead for now, don't just
  log the message and continue as nothing happened.
- Caused me a headache as restart failed, but there was no info, because
  logging died too.

Signed-off-by: David Matějček <[email protected]>
- If something breaks, escalate the problem.
- InstanceDirs have toString now, I noticed it in logs
- EarlyLogger is not used at all

Signed-off-by: David Matějček <[email protected]>
- Removed unused parameter info
- Some javadocs explaining what and why
- GFLauncherLogger doesn't need the synchronization
- Removed unused methods add/remove Jvm logging

Signed-off-by: David Matějček <[email protected]>
@dmatej dmatej added this to the 7.0.19 milestone Oct 7, 2024
@dmatej dmatej requested review from OndroMih, pzygielo, hs536, avpinchuk and a team October 7, 2024 21:10
@dmatej dmatej self-assigned this Oct 7, 2024
@dmatej dmatej requested a review from arjantijms October 7, 2024 21:31
Copy link
Contributor

@hs536 hs536 left a comment

Choose a reason for hiding this comment

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

LGTM

@dmatej dmatej merged commit 76650a9 into eclipse-ee4j:master Oct 8, 2024
2 checks passed
@dmatej dmatej deleted the cleanups branch October 8, 2024 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JDK loggers can initialize the JDK LogManager before GlassFishLogManager is set
2 participants