Make automatic configuration updates configurable per project #1664
Conversation
HannesWell
force-pushed
the
auto-update-project-preference
branch
from
ca7690e to
a2136e0
Compare
HannesWell
force-pushed
the
auto-update-project-preference
branch
2 times, most recently
from
ef9509a to
8f80e1e
Compare
I have now reverted the changes of the |
HannesWell
force-pushed
the
auto-update-project-preference
branch
from
8f80e1e to
07c4386
Compare
Now the change is fully internal and does not need any new methods in any API (that would need |
There was a problem hiding this comment.
Looks good just one thing for the UI I think it must be a radio group with the options:
- Use Workspace default (
- Update Automatically
- Never update automatically
Also from the code it currently looks like if the project has enabled it but it is disabled in the workspace settings no update will ever happen as there is an early exit.
| @@ -40,10 +41,10 @@ public class MavenUpdateConfigurationChangeListener implements IResourceChangeLi | |||
|
|
|||
| @Override | |||
| public void resourceChanged(IResourceChangeEvent event) { | |||
| if(isDisabled()) { | |||
| if(isAutoConfigurationUpdateDisabled()) { | |||
There was a problem hiding this comment.
This must be removed or disbaled in the workspace can't be changed by a project.
| updateProjectConfiguration(outOfDateProjects); | ||
| } | ||
|
|
||
| private boolean isDisabled() { | ||
| public static boolean isAutoConfigurationUpdateDisabled() { |
There was a problem hiding this comment.
I would add an IProject parameter here then it could:
- Check if there is a project config and if yes choose this one unless it is configured to use the workspace default
- fallback to the global default
| public static boolean isAutoConfigurationUpdateDisabled() { | |
| public static boolean isAutoConfigurationUpdate(IProject) { |
| @@ -52,10 +53,12 @@ public void resourceChanged(IResourceChangeEvent event) { | |||
| LOG.error("An error occurred while checking for out-of-date configuration markers", e); | |||
| return; | |||
| } | |||
| outOfDateProjects = outOfDateProjects.stream() // | |||
| .filter(ResolverConfigurationIO::isAutomaticallyUpdateConfiguration).toList(); | |||
There was a problem hiding this comment.
| .filter(ResolverConfigurationIO::isAutomaticallyUpdateConfiguration).toList(); | |
| .filter(MavenConfigurationChangeListener::isAutoConfigurationUpdate).toList(); |
That's right and this was actually by design, based on the assumption that this is mainly used to disable specific projects in their preferences and that the corresponding preferences is committed in the SCM. With that in mind I wanted to keep the preferences simple. But the current behavior is already reflected in the project preference since it is disabled, when the workspace preference is disabled: |
|
I think it depends a bit, e.g. if I have a problem with many projects I might want to have this only on some projects where it works well, or only enable it step by step ... but of course we can add it also if its really a demand it is just that usually project settings take precedence over global settings so this might be a bit of a surprise. |
And degrade problems about out-dated project configuration to severity 'info', if automatic updates are disabled for a project. Fixes eclipse-m2e#1661
HannesWell
force-pushed
the
auto-update-project-preference
branch
from
07c4386 to
2688bc9
Compare
In that case, you are right. But I think the current setup is sufficient for most case (I assume in the workspace it is turned on in most cases as it is default) and I would prefer to wait for further enhancements until there is a real demand for it. |
And degrade problems about out-dated project configuration to severity 'info', if automatic updates are disabled for a project.
Fixes #1661
@laeubi what do you think?
I'm not sure adding API methods is really necessary (for now I just added it for completes/symmetry) but it is probably sufficient to have this internal.