-
Notifications
You must be signed in to change notification settings - Fork 13
Conversation
awisniew90
commented
Aug 28, 2019
- Allow users to set boost properties in maven pom
- Collect boost properties once at the beginning of mojo execution
- Have Liberty generate config variables rather than bootstrap.properties
- Change Liberty server name from BoostServer to defaultServer
- Move password encryption to LibertyServerConfigGenerator (since Liberty specific)
cea3ea2
to
feae3ba
Compare
Signed-off-by: Scott Kurz <skurz@us.ibm.com>
Signed-off-by: Scott Kurz <skurz@us.ibm.com>
Signed-off-by: Scott Kurz <skurz@us.ibm.com>
Signed-off-by: Scott Kurz <skurz@us.ibm.com>
Signed-off-by: Scott Kurz <skurz@us.ibm.com>
Signed-off-by: Scott Kurz <skurz@us.ibm.com>
Signed-off-by: Scott Kurz <skurz@us.ibm.com>
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.
Started making comments, then tried to run it and started making fixes. I first needed these changes: https://github.com/scottkurz/boost-maven/tree/dev_proto_2 plus a ci.maven workaround: https://github.com/scottkurz/ci.maven/tree/488-workaround
The one other issue I still see: checkPropertiesTest in Db2PasswordAesIT/Db2PropertiesIT should be reading vars now not bootstrap.properties.
ServiceLoader<RuntimeI> runtimes = ServiceLoader.load(RuntimeI.class, projectClassLoader); | ||
if (!runtimes.iterator().hasNext()) { | ||
|
||
RuntimeI runtime = null; |
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.
indentation/formatting?
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.
Fixed
private final ExecutionEnvironment env; | ||
private final MavenProject project; | ||
private final Plugin mavenDepPlugin; | ||
|
||
private final String serverName = "BoostServer"; | ||
private final String serverName = "defaultServer"; |
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.
Yeah...good idea.
@@ -21,7 +22,7 @@ | |||
@BoosterCoordinates(AbstractBoosterConfig.BOOSTERS_GROUP_ID + ":cdi") | |||
public class CDIBoosterConfig extends AbstractBoosterConfig { | |||
|
|||
public CDIBoosterConfig(Map<String, String> dependencies, BoostLoggerI logger) throws BoostException { | |||
public CDIBoosterConfig(Map<String, String> dependencies, Properties boostProperties, BoostLoggerI logger) throws BoostException { |
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.
Cross-cutting comment: Since most BoosterConfig(s) don't use all of these.. maybe we should wrap all these in a new BoostConfiguration
type object so we only have to change that once, and then change the places that actually use specific getters. So the next change doesn't hit so many files
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.
Added this in
feae3ba
to
168fda0
Compare
cleanup server name and bug fixes
@scottkurz - I created a BoosterConfigParams class to use when passing parameters to BoosterConfig classes (rather than the separate dependencies and properties). Fixed DB2 tests as well. |
@@ -1,4 +1,4 @@ | |||
#Using profiles to configure failsafe to run certain tests | |||
invoker.goals.1 = clean install -PnoEncoding -Dboost.db.databaseName=myCustomDB -Dboost.db.user=user -Dboost.db.password=password -Dboost.aes.key=test | |||
invoker.goals.1 = clean install |
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.
Did we lose some coverage here?
8c6d6da
to
015d4a4
Compare
Resolved by #361 so closing this as dup. |