Factorize code with ErrorsWrapper and Utils.nullToEmpty #25
Conversation
|
Have you run the mockTests locally ( |
| Branch() { | ||
| } | ||
|
|
||
| @SerializedNames({ "id", "displayId", "type", "latestCommit", "latestChangeset", "isDefault", "errors" }) | ||
| public static Branch create(String id, String displayId, String type, | ||
| String latestCommit, String latestChangeset, boolean isDefault, List<Error> errors) { | ||
| return new AutoValue_Branch(id, displayId, type, latestCommit, latestChangeset, isDefault, | ||
| errors != null ? ImmutableList.copyOf(errors) : ImmutableList.<Error> of()); | ||
| return new AutoValue_Branch(Utils.nullToEmpty(errors), id, displayId, type, latestCommit, latestChangeset, isDefault); |
There was a problem hiding this comment.
The Utils.nullToEmpty(errors) should be the last arg.
There was a problem hiding this comment.
Or maybe not if your implementing from super class?
There was a problem hiding this comment.
AutoValue generated class imposes me to pass errors as first argument.
There was a problem hiding this comment.
Fair enough and wasn't aware of that.
|
|
||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.common.collect.ImmutableMap; | ||
|
|
||
| public class Utils { |
There was a problem hiding this comment.
Can we move this class to say src/main/java/com/cdancy/bitbucket/rest/util/Utils.java? Think it makes more sense there vs putting it under the domain folder.
|
All MockTests pass when I build locally. Travis also seems happy. |
|
@Gawen-pjr disregard that comment. It was in response to where you placed the |
|
@Gawen-pjr very awesome and glad you tackled this next. I was hoping your next PR was going to be this otherwise I was going to hop in and finish the work you started ;) |
|
I'm currently factorizing classes with "Links" field which is also widely used. If you can wait 10 minutes before merging, I'll push the changes. |
|
@Gawen-pjr still reviewing on this end. I'll stop until I have the go-ahead from you that you're finished. |
|
@Gawen-pjr actually ... if you're looking to factor out other variables best to send that in a separate PR. It will make backing out potential changes in less problematic should the need arise. |
|
@Gawen-pjr I like this PullRequest as-is. Lets not add anything else unless it pertains to these changes. Thoughts? |
|
@Gawen-pjr we can do these but no more in this PR!!! ;) I'll allow it because it cleans things up but I generally don't like to review large PR's. They should be as small as possible and address a single issue. |
|
Aw, just pushed last changes. I'll rollback with a reverse commit |
… interface" This reverts commit 1343934.
|
OK. I leave this PR as is. Starting a new PR for Links factorization. Sorry for the disturbances. |
|
Merged. Thanks for the code-cleanup/refactoring/PR @Gawen-pjr !!! |
|
OK. See you in next PR ;-) |
Just propagated usage of ErrorsWrapper interface and Utils.nullToEmpty static method for all domain classes.