Add new field in PullRequest Object #50
Conversation
| */ | ||
| @AutoValue | ||
| public abstract class Properties { | ||
| public abstract Long openTaskCount(); |
There was a problem hiding this comment.
If not nullable we can use long instead of boxed version. Same for resolvedTaskCount.
| public static Reference create(String id, MinimalRepository repository) { | ||
| return new AutoValue_Reference(id != null ? id : "refs/heads/master", repository); | ||
| @SerializedNames({"id", "repository", "displayId"}) | ||
| public static Reference create(String id, MinimalRepository repository, String displayId) { |
There was a problem hiding this comment.
Can we create overloaded constructor here. Previous can default to setting this property to say the id.
There was a problem hiding this comment.
I don't understand what you mean.
There was a problem hiding this comment.
Constructor was the wrong word, my apologies. Overloaded versions of create:
public static Reference create(String id, MinimalRepository repository) {
public static Reference create(String id, MinimalRepository repository, String displayId) {
The first can delegate to the second passing in whatever value id is to displayId.
There was a problem hiding this comment.
Ha ok, but Id and displayId isn't same value from api. Exemple for master branch :
id: "refs/heads/master",
displayId : "master"
There was a problem hiding this comment.
Ok. Then maybe check for non-null and if so split on / and use last index? Something like this might work.
public static Reference create(String id, MinimalRepository repository) {
String displayId = null;
if (id != null) {
String parts [] = id.split("/");
displayId = parts[parts.length];
}
return create(id, repository, displayId);
}
And add a @deprecated annotation to the create(String id, MinimalRepository repository) method.
| CreatePullRequest cpr = CreatePullRequest.create(randomChars, "Fix for issue " + randomChars, fromRef, toRef, null, null); | ||
|
|
||
| System.out.println("---------> CREATED PR: " + cpr); |
There was a problem hiding this comment.
Mind taking out this line while you're here poking around? :)
|
@j0nathan33 thanks splitting up the PR. Added some comments to address. Looks good. Thanks. |
| */ | ||
| @AutoValue | ||
| public abstract class Properties { | ||
| public abstract long openTaskCount(); |
There was a problem hiding this comment.
Can we put a newline between class definition and openTaskCount?
|
|
||
| /** | ||
| * Created by jdoire on 07/03/2017. | ||
| */ |
There was a problem hiding this comment.
Lets remove these Created by comments.
|
@j0nathan33 minor issue with the "overloaded" change but I'll fix that on this end. Thanks for the PR! |
No description provided.