This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | PersistenceType should be better documented, preferably API method | ||
---|---|---|---|
Product: | platform | Reporter: | Jesse Glick <jglick> |
Component: | Window System | Assignee: | mslama <mslama> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | apireviews, jtulach, pbuzek, pzavadsky, tpavek |
Priority: | P2 | Keywords: | API, API_REVIEW_FAST |
Version: | 3.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | ENHANCEMENT | Exception Reporter: | |
Attachments: | Diff for this issue |
Description
Jesse Glick
2003-10-29 20:23:50 UTC
Probably suitable for fast-track review. How is it with this?... is it really necessary now? Marek, it's your area, passing to you so I don't forget about this. What do other think about it? Shall we start FASTREC for this? Regarding default value I think it should stay unchanged ie. P_A. settings files are deleted anyway when they are not used ie. referenced from wstcref or wstcgrp. (So TC must be member of some group or be docked opened or closed to some mode to survive in winsys.) I think this should be done, fast-track review seems to be quite appropriate. You may assign the issue to apireviews@netbeans.org and add API_REVIEW_FAST keyword. Created attachment 12584 [details]
Diff for this issue
Diff is attached. To keep backward compatibility with client property I chose PERSISTENCE_ALWAYS as default persistence type. Going to commit given diff tomorrow if there are no objections. Need also to patch openide/arch/arch-openide-windowsystem.xml#exec-component, openide/api/doc/changes/apichanges.xml, openide/openide-spec-vers.properties, perhaps openide/api/doc/org/openide/doc-files/upgrade.html. Don't forget @since in patch. Also consider printing a brief runtime warning if the method is not overridden to encourage people to specify the behavior using the method; e.g. private static final Set/*<Class>*/ warnedClasses = new WeakSet(); public int getPersistenceType() { if (warnedClasses.add(getClass())) { ErrorManager.getDefault().log(ErrorManager.WARNING, "Note - " + getClass().getName() + " ought to override getPersistenceType() rather than using the client property or accepting the default."); } // as before... } I would print warning only if client property PersistenceType is not null. I think using default behaviour of TopComponent (not overriding getPersistenceType) is ok. Re. the previous comment: Yes, that's OK; I only suggested keeping the warning even for those TC subclasses which do nothing special, because the default value (ALWAYS) is a somewhat dangerous default (even if necessary for backwards compatibility). You need to do plenty of special work to make persistence really work correctly, which many TC's do not need at all (NEVER is fine); and WHEN_OPEN is usually a better choice than ALWAYS since ALWAYS contributes to pollution of the user directory and is rarely of any value to the user. Probably many existing TC's (esp. in third-party modules) are using ALWAYS simply because the authors were not aware of any other possibilities. Keeping the warning on unless the method is overridden simply ensures that people who really wanted ALWAYS will show that explicitly in their code. Ok. I did it as Jesse suggested. Only I do not print warning when it is instance of TopComponent not subclass. I also updated documentation. Looks great, thanks. |