-
Notifications
You must be signed in to change notification settings - Fork 91
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
Add new skip install feature flag for dev mode #1672
Conversation
c55a02b
to
5e4754b
Compare
5e4754b
to
8a8fa51
Compare
liberty-maven-plugin/src/it/dev-skip-install-feature-it/resources/basic-dev-project/pom.xml
Show resolved
Hide resolved
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.
So the DevSkipInstallFeatureTest logic is interesting in that it shows a corner case where you never install the features.
The flow does a Liberty install and then later starts dev mode with "-DskipInstallFeature=true" pointing back to the earlier Liberty install, and confirms we don't install the features.
So it shows it is possible to use this parm in a "bad" way and never get the features installed. You could also do a liberty:create and then "liberty:dev -DskipInstallFeature=true" and also never install the features.
That all said, maybe a more straightforward adaptation of this test would be to do the install feature back when we install Liberty and look for the same log messages:
<configuration>
<serverXmlFile>${project.basedir}/resources/basic-dev-project/src/main/liberty/config/server.xml</serverXmlFile>
...
</configuration>
<executions>
<execution>
<id>install-liberty-server</id>
<phase>compile</phase>
<goals>
<goal>create</goal>
<goal>install-feature</goal>
</goals>
</execution>
</executions>
liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/DevMojo.java
Show resolved
Hide resolved
liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/DevMojo.java
Outdated
Show resolved
Hide resolved
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.
Thx for making the changes.
Fixes #1646
Requires OpenLiberty/ci.common#398
Need to have a similar PR in ci.gradle as well.