You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Adrian Cole <no...@github.com> on 2014/11/16 18:08:08 UTC
[jclouds] Finished partial cleanup of location functions. (#601)
This completes the cleanup @zack-shoylev started in 91f405c9fedf24822015172e9e3c50acbd855d28
I don't mean to single out zack, but we have to stop leaving maintenance for others like me who care about the whole codebase, not just the specific thing I need for one api. I've tried to summarize the sentiment below.
### (Stop) silo'ing fixes!
If you encounter a small problem you need fixed for a downstream api or provider, find some scope to fix uniformly. For example, other classes in the same package, or other methods in the same Api.
Fixing one and leaving the other cleanup for others to address doesn't respect the time of reviewers, or those who merge code at release time. It takes the same time to review a tiny update vs 4 tiny updates. Moreover, a user (or developer) would be unpleasently surprised to find only 1 of 3 edge cases fixed.
When doing bug fixes, or tech-debt removal, can you make it worth it? Could you fix all similar broken windows, or at least more than one? If you can afford time to fix a bug, would spending an extra hour to fix similar ones be tolerable?
Bottom-line, we are all in this together. jclouds doesn't succeed when only new apis, and the partition of shared code they use are maintained.
You can merge this Pull Request by running:
git pull https://github.com/adriancole/jclouds adrian.finish-cleaning-location-functions
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/601
-- Commit Summary --
* Finished partial fix in 91f405c9fedf24822015172e9e3c50acbd855d28.
-- File Changes --
M apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/functions/RegionToAdminEndpointURI.java (28)
M apis/s3/src/main/java/org/jclouds/s3/functions/AssignCorrectHostnameForBucket.java (24)
M apis/s3/src/main/java/org/jclouds/s3/functions/DefaultEndpointThenInvalidateRegion.java (14)
M apis/s3/src/test/java/org/jclouds/s3/functions/AssignCorrectHostnameForBucketTest.java (51)
M apis/s3/src/test/java/org/jclouds/s3/functions/DefaultEndpointThenInvalidateRegionTest.java (22)
M core/src/main/java/org/jclouds/location/functions/RegionToEndpoint.java (15)
M core/src/main/java/org/jclouds/location/functions/RegionToEndpointOrProviderIfNull.java (14)
M core/src/main/java/org/jclouds/location/functions/ToIdAndScope.java (2)
M core/src/main/java/org/jclouds/location/functions/ZoneToEndpoint.java (19)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/601.patch
https://github.com/jclouds/jclouds/pull/601.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by Adrian Cole <no...@github.com>.
Thanks for the review @demobox!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63231826
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1379](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1379/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63232128
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by Andrew Phillips <no...@github.com>.
> @@ -24,7 +24,7 @@
> INSTANCE;
> @Override
> public String apply(Location input) {
> - return input.getId() + ":" + input.getScope();
> + return new StringBuilder().append(input.getId()).append(':').append(input.getScope()).toString();
Doesn't the compiler do this for you automatically?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601/files#r20411473
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by Adrian Cole <no...@github.com>.
>
> -/**
> - * Tests behavior of {@code AssignCorrectHostnameForBucket}
> - */
> -// NOTE:without testName, this will not call @Before* and fail w/NPE during
> -// surefire
Another example of [Cargo Cult](http://en.wikipedia.org/wiki/Cargo_cult) blind copy/pasting (possibly from me). There's no `@Before` method, so no reason why that comment should be valid.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601/files#r20411695
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by Andrew Phillips <no...@github.com>.
>
> -/**
> - * Tests behavior of {@code AssignCorrectHostnameForBucket}
> - */
> -// NOTE:without testName, this will not call @Before* and fail w/NPE during
> -// surefire
I take it this comment was no longer accurate...?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601/files#r20411477
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by Adrian Cole <no...@github.com>.
cherry-picked into master and 1.8.x
I also cherry-picked 91f405c, which @zack-shoylev decided not to do. 1.8.x is the only released current branch and this is core code. Inconsistent cherry-picking causes hell at merge time for the sorry souls who spend time doing that. If we want to help with the concern mesosphere raised about extremely long time to release things, we have to stop setting up traps on our release branch.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63232388
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by Adrian Cole <no...@github.com>.
> @@ -24,7 +24,7 @@
> INSTANCE;
> @Override
> public String apply(Location input) {
> - return input.getId() + ":" + input.getScope();
> + return new StringBuilder().append(input.getId()).append(':').append(input.getScope()).toString();
oops. you are right. will revert
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601/files#r20411678
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1378](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1378/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63228976
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1906](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1906/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63230343
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1907](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1907/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63233569
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #290](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/290/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63231844
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #289](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/289/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63228785
Re: [jclouds] Finished partial cleanup of location functions. (#601)
Posted by Adrian Cole <no...@github.com>.
@demobox thx for the review. I have another coming shortly that narrows down https://issues.apache.org/jira/browse/JCLOUDS-774
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/601#issuecomment-63230983