Architecture Review Opinion
- Issue: 36122
- Submitter:Peter Zavadsky
- Date: 2003-10-29
- Reviewers: Pavel Buzek, Jesse Glick, Jaroslav Tulach, Tomas Pavek
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
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://netbeans.org/bugzilla/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
