You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Roman Coedo <no...@github.com> on 2014/10/30 18:20:36 UTC
[jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes using
@AutoValue (#64)
You can merge this Pull Request by running:
git pull https://github.com/rcoedo/jclouds-labs-aws autovalue
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-aws/pull/64
-- Commit Summary --
* JCLOUDS-750: Add @AutoValue to pom.xml and refactor VaultMetadata
* JCLOUDS-750: Refactor PartMetadata using @AutoValue
* JCLOUDS-750: Refactor some collections using @AutoValue
* JCLOUDS-750: Refactor JobMetadata using @AutoValue
* JCLOUDS-750: Refactor GlacierError using @AutoValue
* JCLOUDS-750: Refactor ArchiveMetadata using @AutoValue
-- File Changes --
M glacier/pom.xml (6)
M glacier/src/main/java/org/jclouds/glacier/domain/ArchiveMetadata.java (82)
M glacier/src/main/java/org/jclouds/glacier/domain/ArchiveMetadataCollection.java (35)
M glacier/src/main/java/org/jclouds/glacier/domain/GlacierError.java (56)
M glacier/src/main/java/org/jclouds/glacier/domain/JobMetadata.java (176)
M glacier/src/main/java/org/jclouds/glacier/domain/MultipartUploadMetadata.java (111)
M glacier/src/main/java/org/jclouds/glacier/domain/PaginatedJobCollection.java (34)
M glacier/src/main/java/org/jclouds/glacier/domain/PaginatedMultipartUploadCollection.java (30)
M glacier/src/main/java/org/jclouds/glacier/domain/PaginatedVaultCollection.java (32)
M glacier/src/main/java/org/jclouds/glacier/domain/PartMetadata.java (28)
M glacier/src/main/java/org/jclouds/glacier/domain/VaultMetadata.java (96)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-aws/pull/64.patch
https://github.com/jclouds/jclouds-labs-aws/pull/64.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Adrian Cole <no...@github.com>.
Good start! you'll probably need something like this to address odd serialized names https://github.com/jclouds/jclouds-labs/blob/master/docker/src/main/java/org/jclouds/docker/config/DockerParserModule.java#L34
Also, please remove builders on any output-only types.
Thanks tons for helping clear our tech debt!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64#issuecomment-61137537
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Adrian Cole <no...@github.com>.
> @@ -121,6 +121,12 @@
> <artifactId>assertj-core</artifactId>
> <scope>test</scope>
> </dependency>
> + <dependency>
> + <groupId>com.google.auto.value</groupId>
> + <artifactId>auto-value</artifactId>
> + <version>1.0-rc2</version>
don't need version as it can be inherited from parent.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64/files#r19622289
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-aws-pull-requests #131](https://jclouds.ci.cloudbees.com/job/jclouds-labs-aws-pull-requests/131/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64#issuecomment-61132085
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Adrian Cole <no...@github.com>.
>
> /**
> * Paginated collection used to store Job listing information.
> */
> -public class PaginatedJobCollection extends IterableWithMarker<JobMetadata> {
> - @SerializedName("JobList")
> - private final Iterable<JobMetadata> jobs;
> - @SerializedName("Marker")
> - private final String marker;
> -
> - @ConstructorProperties({ "JobList", "Marker" })
> - public PaginatedJobCollection(Iterable<JobMetadata> jobs, String marker) {
> - this.jobs = checkNotNull(jobs, "jobs");
> - this.marker = marker;
> - }
> +@AutoValue
> +public abstract class PaginatedJobCollection extends IterableWithMarker<JobMetadata> {
extend ForwardingList. the withMarker stuff can be addressed at the call site (ex. blob list converter). propagating guava where it doesn't add value only makes it easier to break compatibility.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64/files#r19622689
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Adrian Cole <no...@github.com>.
> }
>
> - @Override
> - public int hashCode() {
> - return Objects.hashCode(this.code, this.message, this.type);
> + @SerializedNames({ "code", "message", "type" })
interesting.. caseformat is different between error and other types!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64/files#r19622581
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Roman Coedo <no...@github.com>.
Thanks for the review @adriancole !
I'll address these issues as soon as possible, glad to help.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64#issuecomment-61139102
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Everett Toews <no...@github.com>.
Thanks for the pull request but it's release week in jclouds and that means it's time to clean up the PR queue. This PR will be over 6 months old as of April 1. If you intend to continue work on it, please make a comment by April 2. Otherwise it will be closed on April 3.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64#issuecomment-85718421
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Adrian Cole <no...@github.com>.
>
> - @Override
> - public boolean equals(Object obj) {
> - if (obj == null)
> - return false;
> - if (getClass() != obj.getClass())
> - return false;
> - ArchiveMetadata other = (ArchiveMetadata) obj;
> + public abstract HashCode getTreeHash();
Please keep natural types in value objects. This helps in portability. Ex. `public abstract String sha256TreeHash()`
Move anything that actually requires this being a HashCode object to the call site.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64/files#r19622455
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds-labs-aws #1554](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-aws/1554/) FAILURE
Looks like there's a problem with this pull request
[(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-labs-aws/pull/64#issuecomment-61134149
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Adrian Cole <no...@github.com>.
>
> - public Date getCreationDate() {
> - return (Date) creationDate.clone();
> - }
> + public abstract String getArchiveId();
mind dropping the gets? These are immutable objects. iotw s/getArchiveId/archiveId (same comment everywhere)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64/files#r19622342
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes
using @AutoValue (#64)
Posted by Adrian Cole <no...@github.com>.
>
> +@AutoValue
> +public abstract class ArchiveMetadataCollection extends FluentIterable<ArchiveMetadata>{
switch to ForwardingList. There's nothing about this collection that requires it being a FluentIterable. same comment everywhere.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/64/files#r19622545