You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2015/03/03 17:57:04 UTC
[jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
NOTE: this also adds the 1.1 auto snapshot. This is required for the tests to work. While this is snapshot code, I am fairly certain that generated code (and thus expected functionality) is not expected to change.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/699
-- Commit Summary --
* Adds autovalue/gson builders tests, for sanity.
-- File Changes --
M core/pom.xml (20)
M core/src/test/java/org/jclouds/json/JsonTest.java (84)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/699.patch
https://github.com/jclouds/jclouds/pull/699.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Andrew Gaul <no...@github.com>.
:-1: Agree with @nacx, this is a good change but let's wait for the release.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-77033019
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
@nacx @andrewgaul Good news, everybody! 1.1 is out.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-104045580
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
Reopened #699.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#event-310103618
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
merged and backported
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-104454749
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
Closed #699.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#event-310103610
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Ignasi Barrera <no...@github.com>.
-1
While it only affects the tests, we are opening the door to anyone to use it by mistake, and this must **not** be used until there is a fixed release.
Also, we **only** release the source code, and the tarballs we release must produce the same articafts in each build, at any time. We can't guarantee that by adding a SNAPSHOT dependency. It could also break the tests of an already published release!
I'm -1 to have this merged to master but also understand the value of having these tests to allow us to move forward faster once it is released. I suggest to push a branch with this change (say, auto-1.1) and to add a Jenkins build that verifes it, but SNAPSHOT dependencies can't be merged into master.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-76989512
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
I am not sure what the dedicated branch adds here - do you mean adding it to cloudbees, or just a separate branch under jclouds?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-77104262
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Ignasi Barrera <no...@github.com>.
Athough it is compile-only and won't produce classpath issues in existing deployments, take into account that it *might* produce different code, and that could present conflicts.
I agree this is unlikely to happen, so I'm OK leaving it in 1.9.x if you are confident.
Thx @zack-shoylev @andrewgaul!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-105830383
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Andrew Gaul <no...@github.com>.
Generally I don't think we should upgrade dependencies in minor releases, but since this one is compile-only it should not matter.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-105690540
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
Closed #699.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#event-311206667
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
Alright, but I think it makes more sense to just delay merging this? The snapshot part is to make sure the pr compiles. Also with luck they will release before we do.
________________________________________
From: Ignasi Barrera [notifications@github.com]
Sent: Tuesday, March 3, 2015 11:15 AM
To: jclouds/jclouds
Cc: Zack Shoylev
Subject: Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
-1
While it only affects the tests, we are opening the door to anyone to use it by mistake, and this must not be used until there is a fixed release.
Also, we only release the source code, and the tarballs we release must produce the same articafts in each build, at any time. We can't guarantee that by adding a SNAPSHOT dependency. It could also break the tests of an already published release!
I'm -1 to have this merged to master but also understand the value of having these tests to allow us to move forward faster once it is released. I suggest to push a branch with this change (say, auto-1.1) and to add a Jenkins build that verifes it, but SNAPSHOT dependencies can't be merged into master.
�
Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/699#issuecomment-76989512>.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-76993591
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @zack-shoylev!
We should avoid upgrading versions in patch releases, so I'd say we should revert this commit in 1.9.x. @andrewgaul WDYT?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-104560132
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Ignasi Barrera <no...@github.com>.
Well, a PR is no more than a branch in your fork. I mean pushing that branch to jclouds, so it is more visible and accessible to others, and add it to cloudbees, to have feedback of its status.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-77107673
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Zack Shoylev <no...@github.com>.
@nacx A big point when discussing the next 1.9.x release was making sure we have the builders available, as well as any new code that might use them.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-105635453
Re: [jclouds] Adds autovalue/gson builders tests, for sanity. (#699)
Posted by Ignasi Barrera <no...@github.com>.
Fine by me to leave the PR open (although I'd prefer to have a dedicated branch so others can try and play with it easily). The important thing is to make sure we don't add SNAPSHOT dependencies to master.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/699#issuecomment-77040606