Skip to content
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-6241] Adding 'quickly' profile for fast maven builds with quick profile #5390

Merged
merged 1 commit into from Mar 3, 2023

Conversation

khermano
Copy link
Contributor

Issue: https://issues.redhat.com/browse/WFCORE-6241

My machine:

mvn clean install -DskipTests -> 02:00 min
mvn -Dquickly -> 01:24 min
mvnd -Dquickly -> 46.530 s (7 threads)

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Feb 23, 2023
pom.xml Outdated
<skipTests>true</skipTests>
<enforcer.skip>true</enforcer.skip>
<checkstyle.skip>true</checkstyle.skip>
<assembly.skipAssembly>false</assembly.skipAssembly>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to add assembly.skipAssembly with 'false'? If so, don't add it to the profile, otherwise, I would expect it to be true

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also found the following ones as relevant, could you add them and share your opinion?

<maven.buildNumber.skip>true</maven.buildNumber.skip>
<maven.test.skip>true</maven.test.skip>
<license.skipDownloadLicenses>true</license.skipDownloadLicenses>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forget about license.skipDownloadLicenses, it fails on a full build since the license file is required to allow the galleon-maven-plugin to do the provisioning

Copy link
Contributor Author

@khermano khermano Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assembly.skipAssembly - I saw it in the description so I thought for some reason it needs to be explicitly defined in the pom, sorry about that :)
maven.test.skip - when I was talking with Martin he told me that this option was later removed from the wildly wildfly/wildfly#16416 ... this option was also skipping compiling tests and it brought some failures on CI, but when I tried mvn -Dquickly -Dmaven.repo.local=/tmp/localWfcore it looked okay, with no failures there, so I think in this case should be fine and I will use it, what do you think?
maven.buildNumber.skip - that looks great, thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now, the original configuration runs on my machine mvn -Dquickly in 1:40 min and the new one in 1:20. mvn -Dquickly -Dmaven.repo.local=/tmp/localWfcoreQuickly also looks good without any failures.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank @khermano for pointing out the related WildFly PR.

I tried mvn -Dquickly -Dmaven.repo.local=/tmp/localWfcore it looked okay, with no failures there, so I think in this case should be fine and I will use it, what do you think?

Well, if we want to use the quickly profile to get builds ready to be tested, then we should switch the strategy and remove maven.test.skip and also assembly.skipAssembly since the Zip files generated will be used as artifacts for the WildFly integration test suite.

I'm sorry for the back-and-forth, but yes, now I think we should be consistent with the WildFly full quickly profile and follow the same approach allowing using this profile for building and testing WildFly Core. If you don't need the build for testing, you can manually assembly.skipAssembly and maven.test.skip.

Would you mind removing both to keep the consistency with WildFly full?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at all,
I will remove it right away. I understand that it should be consistent as much as possible.
Thank you very much for your help.

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Mar 3, 2023
@yersan yersan merged commit f5fda13 into wildfly:main Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
2 participants