Hi All,
First at all, let me say that what you are commenting here makes
quite sense to me.
I don't have no idea about IA, so I only want to add a couple of
points of view, regardless of how does it works or how much quality
the IA code has:
I obviously assume that the IA doesn't have any impact in the
current Eclipse OCL Core code. I mean, it's just an non-intrusive
enhancement which works on-top of, or it needs to be configured in
somehow to make it seamlessly work with the OCL Core components .
This is good because we wouldn't have to review EVERY line of code
which may impact in the current OCL Core components behaviour.
In order to try to define a clear line to what (IMHO) should be done
to complete such a promotion in a realistic time frame, I'd focus on
having a well reviewed IA public API, and ideally as small/narrow as
possible. This means that I'd try to "internalize" as much as the
current public API as possible. The reason to do that is that any
design refactoring needed by a bug fix, improvement, a memory
leakage removal, integration with other components or whatever,
could be easily done as long as the code resides in non-published
API. Besides, I think that the main effort we would have to do NOW,
is reviewing such a public API, the smaller public API the IA had
the faster the review would be.
The question now is how much of the current public API could be
"internalized". Well, having a real client I think we could quickly
have a first approach. Every class or interface needed by GMF-T
could be public API, the remaining internal. In the future, as soon
as internal API is required to be public, or we desired to promote
potentially useful API, we could accordingly analyze/review it.
Of course, I don't want to "remove" other suggestions that Ed has
pointed out, since Quality Code necessarily implies most of the
concerns Ed mentioned. I only want to set a minimum of necessary
actions to be done in order to promote the IA as an OCL Core
Component in an acceptable state and time frame and, at the same,
allow to increasingly make the required quality reviews for it.
Finally, I would like to remark (I've not thought about it) the
importance of the new namespace from the point of view of another
stakeholder: The releng :). It could be interesting to avoid
problems like [1] or further changes in the releng stuff
configuration to accomodate new plugins, having a "coherent" or
"uniform" namespace to distinguish Core components from the Tools
one.
[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=370347#c1
Best Regards,
Adolfo.
El 03/02/2012 12:03, Ed Willink escribió:
Hi Michael
That's very encouraging as regards functionality. Just need the
review.
Because the code is largely the work of a single author, and no
other committer has read more than a few lines, I am keen that it
should be reviewed. From my own experience I am aware that code
evolves and not all the revisions that are desirable actually
happen. The IA code has certainly evolved, so I expect that at
least a minor clean up is desirable.
By review, I mean, pay attention to (in decreasing priority)
External API
This is very difficult to change once promoted so it wants to be
right. My understanding is that there is a very small 'facade' so
the external API is small. It's clearly useable, but could or
should it be better? Is it too fat or too thin? Is it extensible?
Internal API
This too can be difficult to change if internal APIs are not
constrained. How much functionality is exposed to arbitrary
derived classes by protected access. Is this necessary? Can it be
shut off? Can a minor refactoring support extension more easily?
Algorithms
Can inefficient O(N*N) algorithms be replaced by O(N)? Can O(N) be
replaced by O(1)? Are results sensibly cached in variables?
Caches
For each field/variable, is the lifetime of the variable
compatible with its validity. i.e. is the status appropriately
updated in response to a Resource change, and is memory leakage
avoided? Resource change is badly handled by OCL at present, so it
may be that for now there is a top level Javadoc mandating total
reconstruction after a change. One day the IA might monitor
resources and automatically update itself.
plugin and other global names
These obviously change. The simplest change is just delete
".examples". Do we want to do something else?
It would be nice if the event plugins went to EMF, but that
doesn't look likely, so they too need review.
It would be nice to have names that can accommodate the pivot
model sometime. I would like to try to partition the code into the
run-time code that performs (re-)evaluation and the meta-run-time
code that maintains the control objects that make IA so good. If
this is possible, then we want corresponding names. Perhaps
org.eclipse.ocl.ecore.impact.runtime
org.eclipse.ocl.ecore.impact.analyzer
I hope that migration of the run-time code to align with the code
generated Java can be done quite easily, since the code generated
Java makes no use of any form of the OCL meta-model; just the
polymorphic Values and polymorphic Domain model for which there is
direct and Reflective Ecore support. Perhaps
org.eclipse.ocl.domain.impact.runtime
Migration of the meta-run-time code will be harder because that
obviously makes use of the OCL meta-model. Perhaps
org.eclipse.ocl.pivot.impact.analyzer
In order to avoid code duplication, code that is independent of
Ecore/UML/Pivot should be in perhaps
org.eclipse.ocl.common.impact
It may also be appropriate to place some declarations independent
of Ecore/UML/Pivot such as extension points in
org.eclipse.ocl.common
We cannot easily use org.eclipse.ocl since that is highly
Ecore/UML dependent.
NB being independent of Ecore does not prohibit use of EObject,
EObject.eClass() etc. I hope that the external API facade can be
Ecore/UML/Pivot independent and so in
org.eclipse.ocl.common.impact.
Naming
Are package, class, method names and to a lesser extent field
names consistent and readable?
CodeGen API
The new direct OCL to Java code generator can be invoked
automatically from genmodel. Addition of some GenAnnotation could
control how an IA instantiation was auto-generated. It would be
good if this relatively simple interface could be thought through
so that it is easy to implement after Juno, if not implemented for
Juno.
Tests
There are many JUnit tests. Is the coverage adequate? Are all of
the tests really useful? Can the tests be combined into one
overall ImpactAnalyzer test suite? Is a memory leakage test
needed?
Layout
Is the code generally readable? I don't expect rigid application
of precise formatting guidelines; just avoid anything too
inconsistent with default Eclipse layout.
Review results
Some review comments can probably be resolved very quickly and so
just done.
Anything affecting API needs to be done as soon as possible.
Anything affecting accuracy should be done soon, but some things
may be hard and so left in a Bugzilla.
It would be good to tackle performance soon as well, but what
cannot be done should go to Bugzilla.
Please branch the code on GitHub and commit your refactorings and
distinctively tagged comments such as // REVIEW or // BUG. Please
avoid mixing name refactorings and review comments in the same
commit, since refactorings tend to produce large deltas;
individual comments will get lost. (One day we might have Gerrit).
Regards
Ed Willink
On 03/02/2012 10:23, Michael Golubev wrote:
Hello,
Below is update from the GMF-T:
For last 3 weeks we were working on the integration of the
ImpactAnalyzer into the GMFT-generated diagrams,
as part of the Bugzilla #368398 -- modeling support for
diagram properties auto-updated based on semantic properties
of the element.
As part of this work we have implemented 50+ tests for IA
(right now they depends on the GMF-T), and manually tested the
generated diagrams (that uses IA) with fairly complex OCL
expressions. We were focusing on the use cases affecting less
than a 100 semantic elements, as it is the primary use case
for diagramming.
So far we have't found any problems and we would like to
promote the work done for #368398 and # into the GMF-T
master.
Even more, in case if IA would be promoted from an examples
to be a part of the MDT OCL, we will consider rewriting the
base diagram updater mechanism for using with IA, because it
is now clear that it would lead to a simpler and more stable
diagram code than the one we are actually generating for
diagram update.
Thus, I am definitely +1 for promoting the IA to the main
OCL distribution.
Regards,
--
![]()
Michael
"Borlander" Golubev
Eclipse Committer (GMF, UML2Tools)
at Montages Think Tank, Prague,
Czech Republic
Montages
AG
Stampfenbachstr. 48, CH-8006 Zürich
tel: +41 44 260 75 57
mob: +420 602 483 463
On Thu, Feb 2, 2012 at 11:55 AM, Axel
Uhl <eclipse@xxxxxxxxxxxxxxxxxx>
wrote:
Philipp,
I can only say that I fully agree with your assessment. We
know what to do: get the IA review done and work on
promoting it from examples/ to plugins/ for the "mature"
ecore implementation.
Best,
-- Axel
On 1/4/2012 11:51 AM, Philipp W. Kutter | Montages AG
wrote:
GMF-T offered to help with the review of the IA and
to promote it to the
MDT OCL main component.
AFAIK the IA only works for the Stable/Old
implementation, which is as
well used in GMF-T currently. Thus while it will of
course help in the
future to extend the IA to the Pivot/New, it would
currently improve the
Stable/Old implementation.
Has the offer been accepted by the MDT OCL project?
I have not seen a
clear answer in this mailing list. I have seen
prototypes of simply
using the example component (thus code
duplication!), and an own GMF-T
IA implemenation, both of them working.
I assume, we all agree that using the IA from the
MDT OCL project in
GMF-T would be the ideal solution.
A clear answer to the offer of the GMF-T would be
very good!
Regards, Philipp
_______________________________________________
mdt-ocl.dev mailing list
mdt-ocl.dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mdt-ocl.dev
_______________________________________________
mdt-ocl.dev mailing list
mdt-ocl.dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mdt-ocl.dev
_______________________________________________
mdt-ocl.dev mailing list
mdt-ocl.dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mdt-ocl.dev
No virus
found in this message.
Checked by AVG - www.avg.com
Version: 2012.0.1913 / Virus Database: 2112/4782 - Release
Date: 02/02/12
_______________________________________________
mdt-ocl.dev mailing list
mdt-ocl.dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mdt-ocl.dev
|
