Skip to content

update version to 1.0.1#1565

Merged
elharo merged 4 commits intomasterfrom
version
Mar 10, 2017
Merged

update version to 1.0.1#1565
elharo merged 4 commits intomasterfrom
version

Conversation

@elharo
Copy link
Copy Markdown
Contributor

@elharo elharo commented Mar 9, 2017

@chanseokoh @briandealwis

Do we need to update this anywhere else?

@chanseokoh
Copy link
Copy Markdown
Contributor

chanseokoh commented Mar 9, 2017

I see a product version "1.0.0" in gcp-repo/metadata.product, but I'm not sure what that means or how it relates to a feature version.

I also see a category version "1.0.0" in gcp-repo/metadata.p2.inf, but I get the feeling that a category is a different kind that may keep its own version. Not sure though.

@briandealwis
Copy link
Copy Markdown
Member

The category in gcp-repo/metadata.p2.inf may be worth changing since it's reflected in the installation UI:

screen shot 2017-03-09 at 4 35 55 pm

The gcp-repo/metadata.product can be left untouched: it won't be visible except to those who know how to use p2 tools.

@chanseokoh
Copy link
Copy Markdown
Contributor

The gcp-repo/metadata.product can be left untouched: it won't be visible except to those who know how to use p2 tools.

Could be useful to put a comment inside gcp-repo/metadata.product.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Mar 9, 2017

PTAL. Updated both.

Comment thread gcp-repo/metadata.p2.inf
units.1.provides.1.namespace=org.eclipse.equinox.p2.iu
units.1.provides.1.name=com.google.cloud.tools.eclipse.category
units.1.provides.1.version=1.0.0
units.1.provides.1.version=1.0.1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you need to change the requires.1.range up above. This .p2.inf format hurts to read. Basically this file associates p2 metadata to the com.google.cloud.tools.eclipse.dist product defined in the gcp-repo/metadata.product file.

  • the initial requires.1 adds a requirement on the product IU for an IU with certain capabilities; think of the namespace as being a the capability type, and the name is the capability identifier. So this requires.1 says the product requires our new category
  • the units.N defines new IUs; the units.N.provides.M describes the capabilities of this IU. In this case this IU provides a category definition. The category values are in the properties.
  • the category IU also has some requirements to pull in a feature

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@briandealwis there is also units.1.version=1.0.0 above. That is left untouched?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know! I think we have to try it and maybe change the unimportant versions to 999.999.999 to indicate that they don't matter.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The units.1.version determines what's shown the user.

diff --git a/gcp-repo/metadata.p2.inf b/gcp-repo/metadata.p2.inf
index 010b3c0..c234b59 100644
--- a/gcp-repo/metadata.p2.inf
+++ b/gcp-repo/metadata.p2.inf
@@ -14,7 +14,7 @@ requires.1.range=[1.0.0,1.0.1]
 requires.1.greedy=true
 
 units.1.id=com.google.cloud.tools.eclipse.category
-units.1.version=1.0.1
+units.1.version=999
 units.1.properties.1.name=org.eclipse.equinox.p2.type.category
 units.1.properties.1.value=true
 units.1.properties.2.name=org.eclipse.equinox.p2.name

produces:
screen shot 2017-03-09 at 11 12 48 pm

@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 9, 2017

Codecov Report

Merging #1565 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1565      +/-   ##
============================================
+ Coverage     71.11%   71.11%   +<.01%     
- Complexity     1335     1349      +14     
============================================
  Files           237      237              
  Lines          9038     9087      +49     
  Branches        774      789      +15     
============================================
+ Hits           6427     6462      +35     
- Misses         2288     2301      +13     
- Partials        323      324       +1
Impacted Files Coverage Δ Complexity Δ
...pse/appengine/validation/AbstractXmlValidator.java 90.32% <0%> (-6.46%) 7% <0%> (ø)
.../tools/eclipse/test/util/project/ProjectUtils.java 87.16% <0%> (-4.76%) 38% <0%> (+14%)
...loud/tools/eclipse/swtbot/SwtBotTreeUtilities.java 17.7% <0%> (-1.05%) 5% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update accae1c...ee0c2af. Read the comment docs.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Mar 10, 2017

PTAL

Copy link
Copy Markdown
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

LGTM. You could narrow the range to [1.0.1,1.0.1] but there's no real compelling reason since we should never possibly pull in the older category from another repository.

@elharo elharo merged commit 3ee17db into master Mar 10, 2017
@elharo elharo deleted the version branch March 10, 2017 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants