-
Notifications
You must be signed in to change notification settings - Fork 130
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
WINDUP-394 Support default value in WindupConfigurationOption #361
WINDUP-394 Support default value in WindupConfigurationOption #361
Conversation
Merged build triggered. |
Merged build started. |
Merged build finished. Test PASSed. |
Test PASSed. |
I don't see the code to use the default value in this commit? I think some changes to WindupCommand are needed as well? |
It is possible I didn't include it in the commit, sorry. Will get to it very soon. |
Any update on this? |
a8a2ddc
to
ebc2c13
Compare
Build triggered. |
I originally intended to simply use it directly - by calling if(... == null ) ... = getDefaultValue() from the Rule itself. But having it done automatically is nicer. So I added what I think could do it (WindupCommand), although not sure. Perhaps that should be done at Forge level? |
Build started. |
Build finished. Test PASSed. |
Test PASSed. |
@Override | ||
public Object getDefaultValue() | ||
{ | ||
if (Boolean.class.isAssignableFrom(this.getType())) |
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.
You probably also want to support "boolean.class"
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.
+1
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.
Done
This looks good to me (it just needs the rebase, IMO). |
ebc2c13
to
d04db33
Compare
Comments processed and rebased. |
Merged build triggered. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Failed Tests: 1org.jboss.windup.rules.apps:rules-java: 1Test FAILed. |
No description provided.