Architecture Review Opinion
- Issue:
36122
- Submitter:Peter Zavadsky
- Date: 2003-10-29
- Reviewers: Pavel Buzek, Jesse Glick, Jaroslav Tulach, Tomas Pavek
- Content
-
Sumary
This document gives review opinion for changes which come with the new window
system implementation. There are a lot of them, abandoning workspaces, introducing
window groups, new version of XML declarations, various API enahncements and API
deprecations.
Maximal care was been taken to support all known client usecases and make old code
to work with new system, however complete compatibility is not possible. On the
other hand due to a significant changes it is recommended that those depreacted
code is revised and adjusted to the new state accordingly, the special is about
XML layer, which is almost necessary to redefine, in order to get the proper layout,
since the chang in layout is significant comparing to the older one.
Decision
Accepted with change requests (Go to implement or commit with completed Technical Change
requests)
There are 5 TCRs that need to be completed before integration. Number of TCAs
was suggested.
Opinion
Especially the following issues were discussed at the inception and commitment reviews.
(Note: the issues that are easy to fix and non controversial are marked as
"will be fixed" and there is no reference issue filed for them)
DOCUMENTATION - Use Cases
T05 The use cases in changes document are not complete. There should
be more use cases, also those already not supported (e.g. for floating
windows) and should be more descriptive (e.g. say what is achieved via
xml and what via programmatic API). Would be nice to have kind of
migration guide for developers (I've found just some UI one on
ui.netbeans.org, not referenced from the changes doc).
Decision: TCA 36943 Complete description of use cases
DOCUMENTATION - Javadoc
T03 Javadoc of Mode interface is not updated - it still advertises
Workspace.createMode.
Decision: already fixed
T04 Javadoc of TopComponentGroup should mention it should never be
implemented anywhere else than in the winsys code.
Decision: already fixed
T06 When will be
openide/api/doc/org/openide/windows/doc-files/api.html updated? This
is the doc that should contain complete description of the new state -
consistent description is needed, not just changes.
(the same) Y08: The overview page in WindowSystem API is empty. It should contain
something. Is at least the WS overview page in openide/javadoc filled
with useful content (links to arch, changes.html, etc.)? Btw. the main
web page does not seem to contain useful links to changes.html, etc.
Decision: TCA 36926 Update windowsystem documentation overview
T08 I've seen a fix in code commented like "Don't do any winsys
manipulation from persistence routines!" Is this mentioned somewhere
in the doc (not calling winsys from persistence code)?
Decision: bug filed (no TCA/TCR): 36944
Y07: WindowSystemImplAPI is defined in arch documents, but it is not
described anywhere. could it be?
Decision: TCA 36927 Describe WindowSystemImplAPI
Document UIManager properties in arch description
Decision: TCR 36925 Document UIManager properties in arch description
API COMMENTS
T01 There is no way to create a mode, the changes.html doc explains
why it is not possible, but would be nice to give alternatives for
former use cases (choosing another existing mode, using default editor
mode, creating dialogs) etc. See also T05.
Decision: covered by T05
T02 Could not mode creation still work in SDI? BTW what is the status
of SDI mode? Is it still supported? Does not seem working wel...
Decision: TCA 36945 keep support for mode creation in SDI
Y09: TopComponentGroup is interface. Why, as it is not supposed to be
implemented, it should be either stated or better be a final class. As
it is for sure that future extensions of the API will like to add
methods like addTopComponent, removeTopComponent, which are currently
missing in the API. If the TCG is going to be kept in current state,
which I do not think is a good idea, please at least mention in javadoc
that nobody should implement the interface.
Decision: will be fixed
Y10: TopComponent.getId has changed from final to non-final, which is
more compatible and cannot cause linking errors. Fine, but is there
really a need for TopComponent.preferedId then? Should not it be better
to extend the constructor with String preferedId parameter? If both
methods are kept, I'd like to see a test to cover the expected behaviour
they are going to have when one of them is overriden, etc.
JG Disagree with Y10 - getId has to be final to have the proper
semantics, and if someone had such a method before, too bad (letting
them override it now would just cause worse problems). And
preferredId() *is* necessary, since the TC needs to state what it
would like its ID to be, but the winsys impl needs to make the final
decision based on the uniqueness constraint.
Decision: TCR 36924 Do not break compatibility of TopComponent
API COMMENTS - PersistenceType
T10 The PersistenceType property of TopComponents seems to but quite
useful. Should it really be private? And should not be even provided
by a real method in TopComponent class?
Y06: Property persitent type is defined as private but I have seen use
of it being suggested at nbdev@. I believe it is not private. Can it
change the stability to at least devel. Could the behaviour be covered
by tests?
JG I agree with T10 & Y06 - almost everyone needs to use PersistenceType
in practice, so it should probably be publicly committed to (not
necessarily right now). In fact I find it very odd that the default is
that top components are persisted even when closed. I rarely want
something which is not visible to be stored on disk and consuming
memory! You should need to ask to keep TC state around when the TC is
closed, IMHO.
Decision: will be fixed
COMPATIBILITY
Y01: The major thing that a bit changes the current policy in NetBeans
is the request of the window system team to add new methods into the
Mode interface and few abstract methods into WindowManager class. Even
this is de jure incompatible change, I support this request as it will
make the API more readable for users and the probability of existing
user code being affect is really low - it would either have to implement
windowsystem or do something really wrong.
Decision: reviewers decided that this is not a problem
T07 Have concerns about functional (not API) compatibility. I've tried
several things (old modules) - and at the best ended with things shown
in the editor area separately without possibility to dock them
somewhere else. Sometimes nothing appears at all, sometimes does not
work (saw complaints by other people on non-adapted modules - e.g.
java view, outline panel, task list). Is the "import" already
implemented? Are there any useful test cases? Should not the user be
allowed to arrange the "imported" components (i.e. dock them somewhere
else than in the editor).
Decision: TCR 36923 Possibility to fix broken import
Y02: I'd like to see the stability category of DND flavors being
described in the arch answers document.
Decision: will be fixed
PERFORMACE
T09 Was there any performance testing/analysis done so far? For things
like reading/storing the config, d&d, switching windows/groups, memory
consumption etc)?
Decision: TCR 36946 basic performance tests for winsys2
PERSISTENCE FORMATS - xml
$userdir/system/Windows2Local/WindowManager.wswmgr has no DOCTYPE
specified. True in other XML files too, pls. correct.
Decision: TCA 36917 Consider specifying DOCTYPE in Windows2{,Local}/** XML files
.wswmgr spelling errors: 'centered-horizontaly' and
'centered-verticaly' (please run a spellchecker on all DTDs!).
"-ally" not "-aly"
Decision: fixed
several issues:
<maximized-mode name="" /> seems a clumsy way to specify that nothing
is maximized. Ditto <active-tc id="" />.
<other permanent="true" /> - "other"?? Weird XML element naming.
"noNameMode_nnn" should probably be "anonymousMode_nnn".
Use of element name <properties/> in tc-ref and group and tc-group
DTDs is confusing to me. Properties of what?
Decision: TCA 36918 Odd formatting choices in winsys2 configuration DTDs
PERSISTENCE FORMATS - Windows2 vs. Windows2Local
Don't understand distinction between Windows2 and Windows2Local
folders. In my userdir (current winsys2 branch build, fresh user dir),
*both* are created on disk (in $userdir/system/), contrary to the
changes doc (3.4.2) which says that Windows2 is read-only. Windows2
for me contains only the Components subdir, which is not in
Windows2Local. But the changes doc goes on to say (later in 3.4.2)
that Components should only be in Windows2. So it contradicts itself.
Please clarify.
Decision: TCA 36922 There should be a way how to make a module work with winsys1
Y03: The stability category of Windows2Local is not described and should
in my opinion be private. Use for that.
Decision: will be fixed
Y04: I suggest to improve the layout of settings on disk to be ready for
possible future return of workspaces or similar concept. I have heard
that it is two weeks task to add them, just the interest from IDE side
is not big, so right now, let's just modify the settings directories to
have Windows2*/Layout/Default/Mode_1.wsmode instead of
Windows2*/Modes/Mode_1.wsmode. This is a simple change, it does not
refer to workspaces, but leaves the door open for adding them later by
introducing more Windows2*/Layout/workspace_name/... than
Windows2*/Layout/Default.
Decision: reviewers decided that this should not be supported (Pavel and
Jesse are for not supporting this, Tomas did not vote, Yarda asked for
mentioning this in minority opinion section)
Y05: I found it nice to split definition of layout from stored
configuration e.g. Windows2 and Windows2Local. Just make sure the window
system never writes to Windows2 otherwise the original problem that
initiated the separation will reappear.
http://www.netbeans.org/issues/show_bug.cgi?id=36642
Decision: TCR 36642 Do not write to Window2 directory
Minority Opinion
Y04: I suggest to improve the layout of settings on disk to be ready for
possible future return of workspaces or similar concept. I have heard
that it is two weeks task to add them, just the interest from IDE side
is not big, so right now, let's just modify the settings directories to
have Windows2*/Layout/Default/Mode_1.wsmode instead of
Windows2*/Modes/Mode_1.wsmode. This is a simple change, it does not
refer to workspaces, but leaves the door open for adding them later by
introducing more Windows2*/Layout/workspace_name/... than
Windows2*/Layout/Default.
Decision: reviewers decided that this should not be supported (Pavel and
Jesse are for not supporting this, Tomas did not vote, Yarda asked for
mentioning this in minority opinion section)
Advisory Information
36944 Document UIManager properties in arch description
Appendixes
Appendix A: Technical Changes Required
36642 Do not write to Window2 directory
36923 Possibility to fix broken import
36924 Do not break compatibility of TopComponent
36946 basic performance tests for winsys2
36925 Document UIManager properties in arch description
Appendix B: Technical Changes Advised
36943 Complete description of use cases
36945 keep support for mode creation in SDI
36917 Consider specifying DOCTYPE in Windows2{,Local}/** XML files
36918 Odd formatting choices in winsys2 configuration DTDs
36922 There should be a way how to make a module work with winsys1
36926 Update windowsystem documentation overview
36927 Describe WindowSystemImplAPI
Appendix C: Reference Material
see
36122 for all related references
