Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Versioning #198

Merged
merged 4 commits into from
Jun 30, 2017
Merged

Versioning #198

merged 4 commits into from
Jun 30, 2017

Conversation

ahgittin
Copy link
Contributor

docs re cleared-up OSGi/semver/maven versioning, as per ML

(this is WIP because the corresponding code is not yet written; however review here could be useful)

Copy link
Member

@drigodwin drigodwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I've suggested some minor changes. Thanks @ahgittin

following Brooklyn's [versioning](versioning.html) rules.
Brooklyn will keep track of that bundle, allow SNAPSHOT-version bundles to be replaced,
and ensure dependent bundles (specified as `brooklyn.libraries` or, for people familiar
with OSGi, the `Required-bundle` manifest header) are available for searching.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably remove for searching as it's not clear what searching is in this context.

For example, `1.0`. `1.0.1` or `1.0.1-20150101`.
In order to do this, Brooklyn requires that the `id:version` string be unique across the catalog:
it is normally an error to add a type if a type with the same `id:version` is present.
Exceptions are if the definition is identical, or if the `version` is noted as a `SNAPSHOT`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exceptions are -> The exceptions to this are

hence the bundle replacement noted above.)

If you are creating an OSGi `MANIFEST.MF` for a bundle that also contains a `catalog.bom`,
you will need to use the mapped rsult (OSGi version syntax) in the manifest,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rsult -> result

@ahgittin
Copy link
Contributor Author

Thanks @drigodwin, all addressed. Should not be merged until after apache/brooklyn-server#743 .

Also added explanation about the 0 minor/patch versions as per @aledsage 's ML comment.

Copy link
Contributor

@geomacy geomacy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One concern about the version comparison.

* Use a `-SNAPSHOT` qualifer suffix on your version when developing
* Increase the version number when making a change to a non-SNAPSHOT type

When adding to the catalog, if no version is supplied, Brooklyn may automatically
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not keen on the "may" here, it seems too woolly - will Brooklyn increment the version or won't it, under what circumstances? Similarly below.

the highest ordered version (see "Ordering" below) in the catalog for the referenced type,
and will thereafter lock the use of that version in that blueprint.
(An exception is where types are co-bundled or an explicit search path is defined;
in the context of evaluating one type, Brooklyn may prefer versions in the same bundle or on the search path.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"may" again

The natural order here is defined as ASCII-lexicographic comparison
for any non-numeric segments (`"a" < "b"`) but numeric order for digit sequences
(`"9" < "10"`), so it does what is usually expected for versions (`1.9` < `1.10`
and `"1.1-rc9-b" < "1.1-rc10-a"`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually puts us at odds with OSGI - they define qualifier comparison to be simply lexicographic. For OSGI, 1.1.0.rc9-b > 1.1.0.rc10-a. If we are converting versions to bundle versions internally we need to match the OSGI version comparison rules. E.g. when brooklyn is working out what the latest version of a bundle is, we don't want Brooklyn choosing a different version than OSGI thinks is the latest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will make this difference explicit -- as noted in apache/brooklyn-server#740 I think this is the right thing to do.

you will need to use the mapped result (OSGi version syntax) in the manifest,
but should continue to use the Brooklyn-recommended syntax in the `catalog.bom`.

For those who are curious, the reason for the Brooklyn version syntax is to reconcile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 a great explanation to have

@ahgittin
Copy link
Contributor Author

Thanks for great review comments. All but one incorporated, including discussion about equality @geomacy .

The one that isn't is the statement about it "may" use co-bundling for searching. This is because we want to get there but currently aren't, or mostly aren't, so being precise here is hard and might become stale. Hopefully we'll soon always do it and can update that word in the docs!

@ahgittin ahgittin changed the title [WIP] Versioning Versioning Jun 30, 2017
@ahgittin
Copy link
Contributor Author

As apache/brooklyn-server#743 is merged I'm merging this.

ahgittin added a commit to ahgittin/brooklyn-docs that referenced this pull request Jun 30, 2017
@asfgit asfgit merged commit 939e2d4 into apache:master Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants