Avoid unnecessary persistence of activity enablements #1084 #1085
Conversation
HeikoKlare
force-pushed
the
issue-1084-startup
branch
6 times, most recently
from
c377b17
to
29e9123
Compare
|
Failing test is documented: #926 |
bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/activities/ActivityManagerEvent.java
Outdated
Show resolved
Hide resolved
HeikoKlare
force-pushed
the
issue-1084-startup
branch
6 times, most recently
from
8b8742f
to
60e445d
Compare
…m#1084 The ActivityPersistenceHelper persists all activity enablements upon every notification about a changed activity enablement it receives. Persistence is, however, only done and thus necessary for activities that are not expression-controlled. So the current implementation performs unnecessary persistence operations. With this change, the ActivityPersistenceHelper only persists the enablements when the enablement of a non-expression-controlled activity has changed. To this end, the received ActivityManagerEvent is extended by the information about whether such an activity was affected. This information is either directly passed to the event construction in case it has already been calculated in the calling context or is calculated lazily when accessing the value. Contributes to eclipse-platform#1084.
HeikoKlare
force-pushed
the
issue-1084-startup
branch
from
60e445d
to
d8bb0db
Compare
|
Looks like this one broke javadoc generation https://download.eclipse.org/eclipse/downloads/drops4/I20230919-1800/compilelogs/platform.doc.isv.javadoc.txt |
|
Also reported here (search for https://github.com/eclipse-platform/eclipse.platform.ui/runs/16931217956 |
|
Sorry for that. I'll provide a fix in a moment. Could we have those checks integrated into mandatory GH Actions checks to have them represented as actual errors of the PR? Or is there a (non-technical) reason for not dong so? |
I think the ones reported there are not related to this PR but pre-existing issues in Javadoc generation, as they appear in every Jenkins log. In particular, the warnings also occur in the build for the PR with the fixes for the broken Javadoc in this PR: https://github.com/eclipse-platform/eclipse.platform.ui/pull/1142/checks?check_run_id=16954953767 |
|
The I-build javadoc generation cares about public API only while the GH action looks at all the sources. We never managed to get that to a clean state to even consider making it fail the build. It's an extra issue (not investigated yet AFAICT) that some javadoc errors are not catched in the Javadoc report but are only in the Maven one which is full with other issues and thus even less people notice these. |
|
Thanks for clarification, @akurtakov! Good to know about that situation and have it in mind when looking at PR check results and logs. |
The
ActivityPersistenceHelperpersists all activity enablements upon every notification about a changed activity enablement it receives. Persistence is, however, only done and thus necessary for activities that are not expression-controlled. So the current implementation performs unnecessary persistence operations.With this change, the ActivityPersistenceHelper only persists the enablements when the enablement of a non-expression-controlled activity has changed. To this end, the received ActivityManagerEvent is extended by the information about whether such an activity was affected. This information is either directly passed to the event construction in case it has already been calculated in the calling context or is calculated lazily when accessing the value.
Contributes to #1084.