Community
Participate
Working Groups
Hello everyone, I found some extension points without documentation and correct definition when i try do any simple bridge to PDT. All given extension points don't have an description and information. We have Eclipse wiki but schema documentation gone. org.eclipse.mylar.monitor.ui.user - no description org.eclipse.mylyn.context.core.bridges org.eclipse.mylyn.context.core.study - no description, attributes questionnairePage and backgroundPage defined as type Java but haven't base type (maybe it's not bug, i don't know). org.eclipse.mylar.context.ui.bridges - no description, attribute lavelProvider is type java but haven't base class/interface.
The base classes also have empty comments. The names isn't always good way for doing documentation.. One sentence per method or all class is insufficiently. For example what given method should do: String org.eclipse.mylyn.context.core.AbstractContextStructureBridge.getParentHandle(String handle) String org.eclipse.mylyn.context.core.AbstractRelationProvider.getSourceId() String org.eclipse.mylyn.context.core.AbstractRelationProvider.getGenericId() List<IDegreeOfSeparation> org.eclipse.mylyn.context.core.AbstractRelationProvider.getDegreesOfSeparation() That's not internal classes, but fragment of public API and gaps in documentation should not take place. Regards, Lukasz
Yes, we need to improve the extension point and public class documentation. Doing so is always at the cost of fixing bugs or improving API, so it's a fine balance and we have been encouraging integrators to look to the reference implementations. In order to help us prioritize adding documentation please feel free to file bugs or send messages to mylyn-integrators and we will add the docs as we answer the questions. Let's continue to use this bug report to gather and prioritize feedback on documenting the extension points and corresponding APIs. Regarding comment#1, AbstractRelationProvider's Javadoc already has a prominent note that this class is currently in flux even though it is being used by some internal clients. To learn more about its use take a look at how the Sandbox Active Search uses it, but we recommended using it with caution since it will change for 3.0. I added docs to AbstractContextStructureBridge. I also added docs to everything mentioned in the description, as well as the missing class constraints (the constraints were already being checked at runtime).
Created attachment 85363 [details] javadocs for the AbstractTaskListFactory class
Created attachment 85364 [details] mylyn/context/zip
Hi, I noticed that the javadoc for AbstractRepositoryConnector.markStaleTasks is broken in the Mylyn 2.x stream for Eclipse 3.3 - it says that it returns a set of changed tasks but the method actually returns a boolean.
(In reply to comment #5) > I noticed that the javadoc for AbstractRepositoryConnector.markStaleTasks is > broken in the Mylyn 2.x stream for Eclipse 3.3 - it says that it returns a set > of changed tasks but the method actually returns a boolean. Thanks, fixed in HEAD.
(In reply to comment #6) > Thanks, fixed in HEAD. Mik, your change is not completely reflect expectations of the current API. Here is a slightly edited variant of what I posted to the integrator's mailing list: --- The markStaleTasks() should return true if there are tasks changed since last synchronization, which also means that queries for the same repository need to be synchronized. Implementation of markStale() should call setStale(true) on each task from collection passed as a tasks parameter to specify if task data need to be updated from the issue tracking system. Note that depends from the issue tracking system capabilities tasks data can be updated right in markStale() method to save additional round trips to server. In this case setStale(true) shouldn't be called, but it is still necessary to return true from markStale() method in order to refresh query results, in case tasks are added or removed from query results.
The documentation should specify the API contract and not how to implement the method.
True, that is why I didn't submitted patch for the javadoc, which still should state that API contract expects setStale() to be called on the changed tasks if those tasks should be included into the following synchronization. Otherwise it is completely unobvious what method should do.
I agree with Eugene, it should at least be stated that setStale(true) has to be called on changed tasks. Besides that the phrasing is still incorrect, e.g. * "Of <code>tasks</code> provided, return all that have changed since last synchronization..." -> the method returns a boolean and not a collection. * The mentioned "collector" is not passed as an argument (maybe this is left over from a previous version?)
Rob: please update that Javadoc accordingly.
Mik, there is patch attached for AbstractTaskListFactory javadocs. Can you please take it or mark as obsolete. Thanks.
please add doc for AbstractRepositoryTaskEditor.validateInput() - What should I do if the input is invalid (there is no return value)?
Mik, can you please review the javadoc contribution I've made? It is attached as a patch. Dennis, as far as I know, only XPlanner connector has some dummy logic in that method. All other connectors rely on the server response after submission, so method is simply empty. Though you could do something like this if you want to validate input on the client side. if(isValid()) { getParentEditor().setMessage(null, IMessageProvider.WARNING); submitButton.setEnabled(true); } else { getParentEditor().setMessage("Field foo is invalid", IMessageProvider.WARNING); submitButton.setEnabled(false); }
(In reply to comment #14) > Mik, can you please review the javadoc contribution I've made? It is attached > as a patch. > > Dennis, as far as I know, only XPlanner connector has some dummy logic in that > method. All other connectors rely on the server response after submission, so > method is simply empty. Though you could do something like this if you want to > validate input on the client side. > > if(isValid()) { > getParentEditor().setMessage(null, IMessageProvider.WARNING); > submitButton.setEnabled(true); > } else { > getParentEditor().setMessage("Field foo is invalid", > IMessageProvider.WARNING); > submitButton.setEnabled(false); > } > Eugene, thanks for the quick reply, unfortunately it seems like you forgot the attachment... I want to do a quick "offline" check on the client side (eg. are all mandatory fields, like issue title, filled? Are the values valid?) to prevent any unnecessary server roundtrips and provide the user with immediate feedback. Of course the data is validated on the server again when submitted and not every thing can be validated on the client side.
(In reply to comment #15) > Eugene, thanks for the quick reply, unfortunately it seems like you forgot the > attachment... I didn't. My first note was addressed to Mik, not you. The example I've posted was all. > I want to do a quick "offline" check on the client side (eg. are all mandatory > fields, like issue title, filled? Are the values valid?) to prevent any > unnecessary server roundtrips and provide the user with immediate feedback. Of > course the data is validated on the server again when submitted and not every > thing can be validated on the client side. Basically all validation is in your hands. If you created fields using some of the factory methods provided by the AbstractRepositoryTaskEditor and its subclasses (i.e. createTextField(), etc), change listener will call AbstractRepositoryTaskEditor.attributeChanged(), which will call validateInput(). For your own controls you may have to call attributeChanged() yourself, and to do the actual validation use code from my previous comment inside of the validateInput() method.
(In reply to comment #13) > please add doc for AbstractRepositoryTaskEditor.validateInput() - What should I > do if the input is invalid (there is no return value)? Rob: when you get a chance please add that Javadoc that Dennis is asking for and take a look at Eugene's patch.
Patch applied. Thanks for the documentation Eugene.
Need to defer: http://wiki.eclipse.org/index.php/Mylyn/3.0_Plan#Deferred_Items
Seems significant for 3.2/Galileo.
Mylyn has been restructured, and our issue tracking has moved to GitHub [1]. We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub. [1] https://github.com/orgs/eclipse-mylyn
