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

WINDUP-1988: Change inputDirectory default for maven plugin to src/main #15

Merged
merged 1 commit into from
Apr 24, 2018
Merged

WINDUP-1988: Change inputDirectory default for maven plugin to src/main #15

merged 1 commit into from
Apr 24, 2018

Conversation

jeichler
Copy link
Contributor

No description provided.

@jsight
Copy link
Member

jsight commented Apr 24, 2018

@jeichler - This will mean that the default configuration will not analyze a multi-module Maven application where the plugin has been configured in the root module. OTOH, it does make the case of a single module, with source in the default location, more logical.

I just wanted to confirm that you have thought about that implication? I'm fine with merging it if you have, so just let me know your thoughts. Thanks!

@jsight
Copy link
Member

jsight commented Apr 24, 2018

Per our discussion this morning, I now realize that the previous default wasn't what I had thought or expected. I had been under the impression that the previous default was the project base directory, but it is actually {basedir}/src/main/java. I agree that src/main is better than that as a default, even though it will still fail in a few cases. It is still less likely to be a problem than the previous default.

Long term, it might be better if we fix the related issues (WINDUP-1824 as well as directory skipping in source mode) in order to better support using the project base directory as the input. Using the project base directory would also fix issues with the reports using incorrect application names and related issues.

For now, I will merge this, as it is a clear improvement over the status quo.

@jeichler - Feel free to add any comments if I've misrepresented anything in the above comment. Also, thanks for taking the time to help me understand the issues involved here.

@jsight jsight merged commit fd47378 into windup:master Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants