-
Notifications
You must be signed in to change notification settings - Fork 3k
Add watchFiles option to allow maven restart on external file changes #49587
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
Conversation
This comment has been minimized.
This comment has been minimized.
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.
Thanks! I haven't looked at the patch yet but here is a quick comment.
List<String> changedPoms = collectChangeFilesFrom(pomFiles); | ||
List<String> changedAdditionalFiles = collectChangeFilesFrom(additionalWatchedFiles); | ||
if (!changedPoms.isEmpty() || !changedAdditionalFiles.isEmpty()) { | ||
logChanges(ListUtils.union(changedPoms, changedAdditionalFiles)); |
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.
I'm not exactly excited at the idea of adding yet another dependency to a Commons jar to our own code, especially since it looks relatively simple to do without, except if I'm missing something?
Just initialize a common ArrayList collector and pass it to both methods. That's more in line with what we have elsewhere.
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.
Haha, I didn't introduce any new dependency :) Someone already snuck it in so I just reused :)
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 a lot of Commons stuff are around due to our Maven dependencies. I will spend some time adding some bans soon.
Signed-off-by: xstefank <[email protected]>
2a6872a
to
d268676
Compare
Status for workflow
|
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.
Thanks @xstefank
This could be documented |
🤦 will do that asap. I will also write a blog post tomorrow. |
Fixes #49558