Bugzilla – Full Text Bug Listing
| Summary: | API Tools view should filter non-API changes (ApiComparator bug for generic methods) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Markus Keller <markus.kell.r> | ||||
| Component: | API Tools | Assignee: | Markus Keller <markus.kell.r> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, markus.kell.r, Vikas.Chandra | ||||
| Version: | 4.6 | Flags: | Vikas.Chandra:
review+
|
||||
| Target Milestone: | 4.6 M6 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| See Also: |
https://git.eclipse.org/r/64594 https://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=a8c6921369ba8801ab25014f3809e9a49e863063 |
||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Markus Keller
This may also be a bug in the ApiComparator when it comes to method signature changes that only affect generics. That's basically all changes in bug 478673. org.eclipse.pde.api.tools.ui.internal.wizards.CompareOperation#run(IProgressMonitor) looks like it already should only consider externally visible APIs: IDelta delta = ApiComparator.compare(.., VisibilityModifiers.API, ..); If a private member becomes public, then now you have an additional API and hence that should be shown in API tool view. However, deltas involving change of private to package access or vice versa are not required. Do you want to filter out such changes? (In reply to Vikas Chandra from comment #2) > If a private member becomes public, then now you have an additional API and > hence that should be shown in API tool view. Of course. > However, deltas involving change of private to package access or vice versa > are not required. Yes that's this bug's subject (filter non-API changes). Steps: - check out org.eclipse.core.filebuffers from master of git://git.eclipse.org/gitroot/platform/eclipse.platform.text.git - Compare With > API Baseline... > I20151124-1000 (or earlier, e.g. 4.5.1) => 4 changes are shown in class TextFileBufferOperation; all for private methods. Example: The non-abstract method org.eclipse.core.filebuffers.manipulation.TextFileBufferOperation.startRewriteSession(ITextFileBuffer) has been added This method has not been added. Only its return type was changed from the raw Map to Map<String, IDocumentPartitioner>. It looks like this bug has been fixed for member types (bug 227803), but it's still present for methods and fields. Non-API top-level types are also handled correctly (skipped). New Gerrit change created: https://git.eclipse.org/r/64594 With https://git.eclipse.org/r/#/c/64594/3 , the API changes in the eclipse.platform.text repo look good. If the private field of a @noextend class is changed to protected, do you consider it as an API change? (In reply to Vikas Chandra from comment #7) > If the private field of a @noextend class is changed to protected, do you > consider it as an API change? No. In an @noextend class, things need to be public to be API. Moving to early m6 Created attachment 259447 [details]
Patch to fix issue mentioned above.
This patch is on top of the gerrit patch which is provided. With this change, private to protected in class like StatusLineContributionItem ( jface code) will be considered as API change and shown in API tool view whereas change of private to protected in class like ToolBarContributionItem will be a non-API change and not shown in API tools view.
(In reply to Vikas Chandra from comment #11) > Created attachment 259447 [details] > Patch to fix issue mentioned above. Yes, I think we need this, but I didn't test the patch yet. Please insert line breaks before the top-level &&, so that the parallel expressions for access/access2 become apparent. (When you do this, you will realize that you should either disable auto-formatting on Save, or at least enable "Line Wrapping > Never join already wrapped lines".) Shouldn't each first occurrence of this.currentDescriptorRestrictions actually be this.initialDescriptorRestrictions? > Shouldn't each first occurrence of this.currentDescriptorRestrictions
> actually be this.initialDescriptorRestrictions?
Please release once you've verified this is OK.
I've uploaded https://git.eclipse.org/r/#/c/64594/4 with the patch and the change from comment 12. Manually verified with the protected ActionContributionItem#shortenText(...). Gerrit change https://git.eclipse.org/r/64594 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=a8c6921369ba8801ab25014f3809e9a49e863063 Thanks Markus ! verified in Version: Neon (4.6) Build id: I20160314-2000 |