Replace project APIs using antiquated Equinox resolver's VersionRange #1340
Conversation
| VersionRange getVersion(); | ||
| // TODO: alternatively name it getHostVersionRange() ? |
There was a problem hiding this comment.
Since only the return type changes from org.eclipse.osgi.service.resolver.VersionRange to org.osgi.framework.VersionRange to we have to find a new name for the method.
The OSGi spec only talks about the version of a requirement (e.g. host, package or bundle) although technically it can be a version range. With that in mind I found naming the new method just getVersion() a good choice, especially since there is no need to reflect the exact return type in a method name. The method name should reflect the semantic of a method.
Alternatively we could name it for example getHostVersionRange(). But not only would this 'duplicate' the return type information from the method signature but also the information of the containing class.
The same applies for IPackageImportDescription.getVersion() and IRequiredBundleDescription.getVersion().
There was a problem hiding this comment.
What about getRange()? or we can folow the record semantics and name it versionRange() even though it is a bit unusual at the first place, I found this often convenient to have an interface and use a record as the implementation, then these interface methods are automatically implemented.
There was a problem hiding this comment.
For me getRange() is not more clear than getVersion().
The point about records is true and actually I like using records but since the implementations are not records we don't have an immediate benefit from that and would would just be a single method with a different style.
But I still find this idea interesting and created #1341 to explore it further.
In the meantime my suggestion is to continue with getVersion(). Since we have enough time to complete that in this release cycle we can immediately replace that method e.g. by version() or versionRange() in #1341.
HannesWell
force-pushed
the
replace-resolver-VersionRange-APIs
branch
from
b4c63b0
to
56322aa
Compare
| @@ -2,7 +2,7 @@ Manifest-Version: 1.0 | |||
| Bundle-ManifestVersion: 2 | |||
| Bundle-Name: %pluginName | |||
| Bundle-SymbolicName: org.eclipse.pde.doc.user; singleton:=true | |||
| Bundle-Version: 3.15.300.qualifier | |||
| Bundle-Version: 3.15.400.qualifier | |||
There was a problem hiding this comment.
Btw. does anybody know the policy when to bump the minor and when to bump the micro/service version of a doc bundle? Because it is usually not used from code (isn't it?) that bundle itself does not really have an API and versioning according to that not really possible, I think?
There was a problem hiding this comment.
At one point in the history is was 3.14.2200.qualifier suggesting 22 +100 increments. I doubt there is ever a compelling reason to increment the minor version.
There was a problem hiding this comment.
That's right. Just incrementing the service version forever does not look suitable.
We could just increment the minor version when new API is added or some is removed after the deprecation period. But this could mean that the minor version also raises relatively quickly since the doc is some kind of an aggregator. Again on the other hand it still wouldn't be higher than the aggregator features since they are incremented every time.
There was a problem hiding this comment.
One could think about just increment the minor version on each release and therefore reset the micro version to zero.
There was a problem hiding this comment.
That would be possible, but for this change I think it is sufficient if we agree to bump the minor version.
There was a problem hiding this comment.
What if we align the version of PDE Doc bundle with the version of org.eclipse.pde feature? The argument could be: PDE Doc bundle content should be in sync with a content of main PDE feature.
There was a problem hiding this comment.
Fair point.
Although this means that I had to bump the minor version of the o.e.pde feature.
I assumed the p2 version checker would suggest to bump a feature's minor version if a plugin's minor has changed. 🤔
But maybe I confused this.
HannesWell
force-pushed
the
replace-resolver-VersionRange-APIs
branch
2 times, most recently
from
61a07b6
to
d5079fd
Compare
Provide alternatives that are using org.osgi.framework.VersionRange and deprecate the existing methods for removal. Part of eclipse-pde#1069
HannesWell
force-pushed
the
replace-resolver-VersionRange-APIs
branch
from
d5079fd
to
2db5265
Compare
|
As a follow up I have created #1343 to replace the methods deprecated here in the PDE code base. If objections are raised I plan to submit this tomorrow Tuesday (European) evening. |
|
Thanks to everyone involved. |
THe 'since' value was set wrong in eclipse-pde#1340.
The 'since' value was set wrong in eclipse-pde#1340.
The 'since' value was set wrong in eclipse-pde#1340.
The 'since' value was set wrong in #1340.
The 'since' value was set wrong in eclipse-pde#1340.
Provide alternatives that are using
org.osgi.framework.VersionRangeand deprecate the existing methods for removal.This is extracted from #1163 handling only the
simplecases.Part of #1069.