-
Notifications
You must be signed in to change notification settings - Fork 145
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
Smaller cleanups #25172
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]>
Signed-off-by: David Matějček <[email protected]>
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]>
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]>
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]>
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]>
hs536
approved these changes
Oct 8, 2024
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.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.