-
Notifications
You must be signed in to change notification settings - Fork 465
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
[WFCORE-4917]: Experimental - Provide a cool boot banner. #5821
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
...product.conf/content/modules/system/layers/base/org/jboss/as/product/wildfly-core/module.xml
Outdated
Show resolved
Hide resolved
3568288
to
1ef65d1
Compare
@yersan all should be fixed :) |
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.
Changes look good to me, just a minor variable removal nit. However, they would need also a consensus about how WildFly full is going to handle a banner. I am assuming full is going to overwrite both banner.txt
file and the galleon package name
@bstansberry / @pferraro / @ehsavoie could you add your feedback here about how we plan to handle it in WildFly full? thanks
@@ -15,6 +15,7 @@ | |||
<prop name="--internal-remove-config" value=""/> | |||
</props> | |||
<packages> | |||
<package name="banner" optional="true"/> |
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.
My initial concern here was more about how this will affect WildFly full. If this package were enabled in full, we would get the WildFly Core banner in WildFly full. I don't know what are the plans for a banner in WildFly full, if there is one, I guess it will overwrite the banner.txt with its banner and it will use the same package.
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.
We can use a WildFly banner instead
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.
Yes the idea is to provide the same package with a WildFly Full banner
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.
@bstansberry Do yo agree with that ?
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.
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.
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.
I haven't approved wildfly/wildfly#17703 yet, but the basic design there that's reflects this PR is fine with me.
I like that there is no banner.txt here in wildfly-core. It's not needed for how we use the wildfly-core feature pack and can only result in problems.
Core -> Full Integration Build 13208 outcome was UNKNOWN using a merge of ef0297b |
Core -> Full Integration Build 13449 outcome was UNKNOWN using a merge of ef0297b |
Core -> WildFly Preview Integration Build 13271 outcome was UNKNOWN using a merge of ef0297b |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Core -> WildFly Preview Integration Build 13387 outcome was FAILURE using a merge of b9096b1 |
Core -> Full Integration Build 13330 outcome was FAILURE using a merge of b9096b1 |
Core -> Full Integration Build 13572 outcome was FAILURE using a merge of b9096b1 |
20688b3
to
2eb6a7e
Compare
Core -> Full Integration Build 13609 outcome was FAILURE using a merge of b25e44a |
Core -> Full Integration Build 13363 outcome was FAILURE using a merge of b25e44a |
Core -> WildFly Preview Integration Build 13420 outcome was FAILURE using a merge of b25e44a |
* Add support for a cool ASCII art banner via a banner.txt in the $JBOSS_HOME/bin directory. Jira: https://issues.redhat.com/browse/WFCORE-4917 Signed-off-by: Emmanuel Hugonnet <[email protected]>
@yersan the handling in full WF is fine; I've approved wildfly/wildfly#17703 |
Thanks @ehsavoie / @bstansberry |
Jira: https://issues.redhat.com/browse/WFCORE-4917
https://issues.redhat.com/browse/WFLY-19116
Required for wildfly/wildfly#17703