Add function for PullRequestService to download raw content of pull r… #481

huydx · 2016-12-02T05:19:08Z

…equest

We already has an issue here #6
And as @willnorris said it's better to use Contents API to download, but Contents API seems to support blob or file content which is a part of a repository git tree only. (not include pull request)

I want to support patch/diff media type in pull request also (which may not be supported even in future release).
So i added method GetRaw to PullRequestService separate from original Get method (to maintain backward compatibility)

googlebot · 2016-12-02T05:19:10Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

googlebot · 2016-12-02T05:20:20Z

CLAs look good, thanks!

huydx · 2016-12-02T05:33:55Z

I see the travis fails but not sure what does that mean, do anybody know?

gmlewis · 2016-12-02T05:43:01Z

Yes. Please run gofmt on both pulls.go and pulls_test.go.

huydx · 2016-12-02T06:11:50Z

thanks @gmlewis, ran and pushed :)

dmitshur

I've left a review with suggestions for how to make this PR better, and general tips and pointers about Go style. But we should probably come to an agreement that it's worth adding GetRaw before you spend a lot of time addressing the code and style feedback.

I haven't thought too much about it at a higher level yet. Generally, it's better to have fewer methods that map directly to the GitHub API, so we should only add this new one if there's no good other way to resolve this task.

dmitshur · 2016-12-02T16:10:06Z

github/pulls.go

@@ -134,6 +135,29 @@ func (s *PullRequestsService) Get(owner string, repo string, number int) (*PullR
 	return pull, resp, err
 }

+// Get pull request raw format (diff or patch)


To follow Go style, the comment should begin with the symbol name. It's GetRaw, so it should be // GetRaw gets .... Also, it'd be nice to end the sentence with a period.

Doc comments work best as complete sentences, which allow a wide variety of automated presentations. The first sentence should be a one-sentence summary that starts with the name being declared.

Source: https://golang.org/doc/effective_go.html#commentary

dmitshur · 2016-12-02T16:10:57Z

github/github.go

@@ -52,6 +52,11 @@ const (
 	// https://developer.github.com/changes/2014-12-09-new-attributes-for-stars-api/
 	mediaTypeStarringPreview = "application/vnd.github.v3.star+json"

+	//https://developer.github.com/v3/media/


To follow Go style and for consistency, there should be a single space at the beginning of this comment.

See https://dmitri.shuralyov.com/idiomatic-go#comments-for-humans-always-have-a-single-space-after-the-slashes for full rationale.

dmitshur · 2016-12-02T16:16:21Z

github/github.go

@@ -154,6 +159,17 @@ type UploadOptions struct {
 	Name string `url:"name,omitempty"`
 }

+type MediaRawType int


This (and a few more below) is an exported package-level symbol. Go style suggests:

All top-level, exported names should have doc comments, as should non-trivial unexported type or function declarations.

Source: https://github.com/golang/go/wiki/CodeReviewComments#doc-comments

dmitshur · 2016-12-02T16:18:19Z

github/pulls.go

+		req.Header.Set("Accept", mediaTypeDiff)
+	} else if opt.Type == Patch {
+		req.Header.Set("Accept", mediaTypePatch)
+	}


Consider using a switch here, it would look nicer.

dmitshur · 2016-12-02T16:21:40Z

github/github.go

+	//https://developer.github.com/v3/media/
+	mediaTypeDiff  = "application/vnd.github.v3.diff"
+	mediaTypePatch = "application/vnd.github.v3.patch"
+	mediaTypeSha   = "application/vnd.github.v3.sha"


mediaTypeSha already exists, it's mediaTypeV3SHA. It's probably better to add the other 2 in that same general area.

Also, the reason it's SHA and not Sha is because Go style uses consistent case for acronyms. See here for rationale.

dmitshur · 2016-12-02T16:25:48Z

github/pulls.go

+		return "", nil, err
+	}
+
+	if opt.Type == Diff {


opt is a pointer, but you never check if its value is nil.

Usually a pointer is used when nil means using default options. Either do that, or make it a value.

Making it a value makes more sense, since I can't think of a good "default" value for MediaRawTypeOption.

dmitshur · 2016-12-02T16:26:57Z

github/pulls.go

@@ -134,6 +135,29 @@ func (s *PullRequestsService) Get(owner string, repo string, number int) (*PullR
 	return pull, resp, err
 }

+// Get pull request raw format (diff or patch)
+func (s *PullRequestsService) GetRaw(owner string, repo string, number int, opt *MediaRawTypeOption) (string, *Response, error) {


I think MediaRawTypeOption should be named GetRawOptions. See other funcs that take an options struct, that's the pattern they follow.

dmitshur · 2016-12-02T16:27:42Z

github/pulls.go

+		return "", resp, err
+	}
+
+	return string(ret.Bytes()), resp, err


string(ret.Bytes()) can be simplified to ret.String(). But are you sure that returning a string rather than []byte makes the most sense?

huydx · 2016-12-03T09:23:45Z

@shurcooL Thanks for your nice comments, i agree with all of your comments and will fix after we agree on should we create new method or not.
I think of 3 options here

Reuse original Get method by put an option there and new field into PullRequest struct called raw (which is string type). This option is not good in term of github API mapping because actually there is not any raw in the real response
Create new GetRaw method as I did
Write generic DownloadRaw method as below

func DownloadRaw(type {}interface, rawOption *RawOption) {

}

type could be PullRequest, Commit or types may support patch or diff in future.

HDYT?

dmitshur · 2016-12-04T09:30:58Z

@shurcooL Thanks for your nice comments, i agree with all of your comments and will fix after we agree on should we create new method or not.

Sure thing. Sounds good!

Of the three ideas you've listed, I think first one is pretty much not good and not viable. It'd be a breaking API change, and it would not be worth to touch such an important/common endpoint with an option that's unlikely to be used very often.

Ideas 2 and 3 seem like reasonable solutions. I can't seem to find one I think is better.

Idea 2 is a smaller step and more concrete, and you need it. Idea 3 seems like a better choice in the long run, but only if all other types are implemented and useful in the future. That said, I'd prefer to optimize for short run, because it's concrete and more likely to be useful.

We can also look at the API for downloading contents and see how this can compare and fit in: https://godoc.org/github.com/google/go-github/github#RepositoriesService.DownloadContents.

I wonder if we're missing any other ideas for how to do this. So far I can't think of anything else, other than doing a manual request with NewRequest and Do.

That's all I got for now. Thoughts from others?

huydx · 2016-12-06T01:05:49Z

@shurcooL Thanks for throughout comment, I've take a look at download contents api, but the implementation is mapping 1-1 to separate API at github endpoint, so this PR may be not fit to go in there. As an user, i think GetRaw method is quite easy to reasoning, so keep as it is seems fine to me :).

dmitshur · 2016-12-06T01:27:13Z

All right, that sounds good to me.

@willnorris or @gmlewis, what do you think of the currently proposed approach of adding GetRaw method to PullRequestsService in order to provide ability to fetch the pull request with different media types?

I think your feedback and approval to go ahead with that is needed to make progress here. If accepted, then @huydx can work on addressing the review comments.

gmlewis · 2016-12-06T17:20:12Z

Yes, I agree that option 3 above is probably best for the long-haul but also will involve a great deal of work or at the very least is a non-trivial change.

I also agree that since this is needed now, option 2 (this PR) seems like the best way to move forward.

@willnorris has the final say on a change like this, and he may also have a better idea.

In the meantime, though, @willnorris has said that if @shurcooL and I agree, then we can move forward with that decision. I therefore recommend letting this PR sit for one or two more days to allow @willnorris to respond, and then go ahead and move forward with the excellent review comments provided by @shurcooL. Thank you, @huydx!

huydx · 2016-12-13T01:04:21Z

gentle ping @willnorris

dmitshur · 2016-12-13T02:21:30Z

@huydx, see last paragraph in the comment above:

In the meantime, though, @willnorris has said that if @shurcooL and I agree, then we can move forward with that decision. I therefore recommend letting this PR sit for one or two more days to allow @willnorris to respond, and then go ahead and move forward with the excellent review comments provided by @shurcooL. Thank you, @huydx!

It has been a few days, and there's no pushback. So feel free to address the feedback and we'll be able to merge this.

huydx · 2016-12-13T13:43:04Z

fixed @shurcooL . Return of GetRaw function now is []byte to avoid unncessary memory copy (which is not transparent from user view).

dmitshur · 2016-12-13T17:10:18Z

.gitignore

@@ -1 +1,2 @@
 *.test
+.idea


Please get rid of this, I don't think it belongs here. If you need to ignore .idea files, you can add it to your personal .git/info/exclude file, which doesn't get committed. See https://help.github.com/articles/ignoring-files/#explicit-repository-excludes.

dmitshur · 2016-12-13T17:11:30Z

github/github.go

@@ -154,6 +156,19 @@ type UploadOptions struct {
 	Name string `url:"name,omitempty"`
 }

+// MediaRawType represents raw return of a request instead of json format


Add a missing period at end of sentence and spell it as JSON, not json, since it's an acronym.

dmitshur · 2016-12-13T17:15:56Z

github/github.go

+	Patch
+)
+
+// GetRawOptions specifies the optional parameter when user want to get raw presentation of a response instead of json format


I think you can simplify the docs to just:

// GetRawOptions specifies the parameters to the PullRequestsService.GetRaw method.

The parameter is not optional, so it's not accurate to say that it is.

Your comment is also missing a period at the end of the sentence.

dmitshur · 2016-12-13T17:17:29Z

github/github.go

+)
+
+// GetRawOptions specifies the optional parameter when user want to get raw presentation of a response instead of json format
+type GetRawOptions struct {


What do you think about shortening the name to RawOptions?

If you look at other structs in this package that have Options suffix, they're usually named after the general area of interest, not the exact method name.

dmitshur · 2016-12-13T17:18:41Z

github/pulls.go

+	u := fmt.Sprintf("repos/%v/%v/pulls/%d", owner, repo, number)
+	req, err := s.client.NewRequest("GET", u, nil)
+	if err != nil {
+		return []byte{}, nil, err


No need to allocate an empty byte slice when returning an error here, just return a nil byte slice.

return nil, nil, err

dmitshur · 2016-12-13T17:19:38Z

github/pulls.go

+		req.Header.Set("Accept", mediaTypeV3Diff)
+	} else if opt.Type == Patch {
+		req.Header.Set("Accept", mediaTypeV3Patch)
+	}


Two things here.

A switch statement would be a good fit here.

How about handling when opt.Type value is not any of the supported types? I think returning an error would be most appropriate. Currently, it would just perform undefined actions.

dmitshur · 2016-12-13T17:19:57Z

github/pulls.go

+	ret := new(bytes.Buffer)
+	resp, err := s.client.Do(req, ret)
+	if err != nil {
+		return []byte{}, resp, err


Return a nil byte slice instead of allocating an empty one here also.

dmitshur · 2016-12-13T21:25:37Z

github/pulls_test.go

+func TestPullRequestService_GetRawPatch(t *testing.T) {
+	setup()
+	defer teardown()
+	var rawStr = "@@patch content"


This can be const, no need for var.

dmitshur · 2016-12-13T21:25:43Z

github/pulls_test.go

+func TestPullRequestService_GetRawDiff(t *testing.T) {
+	setup()
+	defer teardown()
+	var rawStr = "@@diff content"


This can be const too.

huydx · 2016-12-14T01:31:00Z

@shurcooL thanks for your patience, fixed for 2nd round. Added test for invalid option as well.

dmitshur · 2016-12-14T05:02:49Z

github/pulls.go

+	case Patch:
+		req.Header.Set("Accept", mediaTypeV3Patch)
+	default:
+		return nil, nil, fmt.Errorf("Unsupported raw type %d", opt.Type)


Style issue. Error strings should not be capitalized.

Rationale: https://github.com/golang/go/wiki/CodeReviewComments#error-strings

dmitshur · 2016-12-14T05:07:37Z

github/pulls_test.go

+		fmt.Fprint(w, rawStr)
+	})
+
+	ret, _, err := client.PullRequests.GetRaw("o", "r", 1, RawOptions{Patch})


Client code will not be able to write github.RawOptions{github.Patch}, because that'd cause a vet issue of composite literal using unkeyed fields.

They'd have to write github.RawOptions{Type: github.Patch}.

On the one hand, keeping the RawOptions struct means we can add additional options in the future, but on the other hand, it's more verbose than just passing a RawType parameter without a struct.

Just making an observation here. Not asking to change anything.

dmitshur · 2016-12-14T05:15:04Z

Thanks for addressing the feedback @huydx.

I've left a few more minor comments. Overall, this looks good so far. I'll review it more carefully and leave my formal LGTM tomorrow or so.

@gmlewis, now that this PR has almost the final form, do you wanna take another look and tell us how it looks to you?

gmlewis

This LGTM after @shurcooL's comments and a couple minor ones of mine are addressed.
Thank you, @huydx and @shurcooL!

gmlewis · 2016-12-14T06:54:51Z

github/github.go

+	Patch
+)
+
+// RawOptions specifies parameters when user want to get raw format of a response instead of JSON.


nit: s/want/wants/
Also, if you browse around this repo, you will see that the comments attempt to stay within 80 columns although obviously this is not strictly necessary for Go programming. But just to keep things consistent, it might be nice to break this line.

huydx · 2016-12-14T10:18:00Z

PTAL~ @gmlewis @shurcooL

…equest

huydx · 2016-12-19T14:53:39Z

PTAL @shurcooL

dmitshur

A few really minor comments to make the tests better.

LGTM.

Thanks @huydx!

dmitshur · 2016-12-19T16:58:23Z

github/pulls_test.go

+	defer teardown()
+	_, _, err := client.PullRequests.GetRaw("o", "r", 1, RawOptions{100})
+	if err == nil {
+		t.Error("PullRequests.GetRaw should return error")


If err == nil, it doesn't make sense to continue to the next check that looks at err.Error().

So, change t.Error to t.Fatal here.

dmitshur · 2016-12-19T16:58:46Z

github/pulls_test.go

+
+	ret, _, err := client.PullRequests.GetRaw("o", "r", 1, RawOptions{Diff})
+	if err != nil {
+		t.Errorf("PullRequests.GetRaw return error: %v", err)


Here too, this should be t.Fatalf.

dmitshur · 2016-12-19T16:58:54Z

github/pulls_test.go

+
+	ret, _, err := client.PullRequests.GetRaw("o", "r", 1, RawOptions{Patch})
+	if err != nil {
+		t.Errorf("PullRequests.GetRaw return error: %v", err)


dmitshur · 2016-12-19T18:19:44Z

github/github.go

+
+const (
+	Diff RawType = 1 + iota
+	Patch


This causes 2 golint issues:

github.go:163:2: exported const Diff should have comment (or a comment on this block) or be unexported github.go:164:2: exported const Patch should have comment (or a comment on this block) or be unexported

dmitshur · 2016-12-19T18:29:27Z

I've applied the minor changes above (also swapped order of GetRaw{Diff,Patch} tests to be consistent) and merged this PR as 7edde48. Thank you @huydx.

huydx · 2016-12-20T01:11:54Z

thanks~ @shurcooL

tamird · 2016-12-20T17:56:51Z

I just noticed this change, and was about to apply the following diff to my code:

diff --git a/pkg/cmd/github-pull-request-make/main.go b/pkg/cmd/github-pull-request-make/main.go
index 7d917bee6..16e1668f3 100644
--- a/pkg/cmd/github-pull-request-make/main.go
+++ b/pkg/cmd/github-pull-request-make/main.go
@@ -32,7 +32,6 @@ import (
 	"go/build"
 	"io"
 	"log"
-	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -134,16 +133,17 @@ func main() {
 		return
 	}
 
-	resp, err := http.Get(*currentPull.DiffURL)
+	diff, _, err := client.PullRequests.GetRaw(
+		*currentPull.Head.Repo.Organization.Name,
+		*currentPull.Head.Repo.Name,
+		*currentPull.Number,
+		github.RawOptions{Type: github.Patch},
+	)
 	if err != nil {
 		log.Fatal(err)
 	}
-	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		log.Fatalf("http.Get(%s).Status = %s", *currentPull.DiffURL, resp.Status)
-	}
 
-	pkgs, err := pkgsFromDiff(resp.Body)
+	pkgs, err := pkgsFromDiff(strings.NewReader(diff))
 	if err != nil {
 		log.Fatal(err)
 	}

However, the fact that GetRaw returns a string is kind of unfortunate - returning the bytes.Buffer directly would be strictly more flexible, and more efficient.

dmitshur · 2016-12-20T18:04:18Z

Just for the sake of having a complete picture, can you show what pkgsFromDiff does with the io.Reader?

This API is still very new and not many people can be depending on it already. If we can agree on an improvement, changing it is a lot more viable than breaking an API that has been out for years.

tamird · 2016-12-20T18:06:26Z

https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/github-pull-request-make/main.go#L62

…

On Tue, Dec 20, 2016 at 1:04 PM, Dmitri Shuralyov ***@***.***> wrote: Just for the sake of having a complete picture, can you show what pkgsFromDiff does with the io.Reader? This API is still very new and not many people can be depending on it already. If we can agree on an improvement, changing it is a lot more viable than breaking an API that has been out for years. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#481 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPMJVbrcqdCCayKLY9U9eUi6BotxJks5rKBikgaJpZM4LCNY_> .

dmitshur · 2016-12-20T18:39:00Z

I see. So you're wrapping the reader in a bufio.NewReader, and doing that for its ReadLine method.

If you wanted to maximize efficiency, all that could be rewritten to loop over the string with strings.IndexByte(s, '\n'), since buffering a *bytes.Buffer or string is not needed.

But, still, returning *bytes.Buffer instead of buf.String() does seem more flexible and equally friendly... If @gmlewis is in agreement, we'll take a PR for it.

tamird · 2016-12-20T18:45:20Z

heh, maximizing efficiency would certainly mean not allocating a string to begin with. In any case, this isn't terrible (nobody is writing high performance GH API tooling), so whatever is otherwise the convention in this repo should determine the shape of this API.

dmitshur · 2016-12-20T18:51:33Z

so whatever is otherwise the convention in this repo should determine the shape of this API.

There wasn't, we had to come up with something new. That's why this is still open for discussion. Most other methods in this library return structs like *PullRequest or *Issue.

Edit: There wasn't much, but:

go-github already has a few endpoints that return text, such as Markdown, Octocat and Zen, and they all use string.

The PR originally started by returning []byte, but I made the suggestion to use string instead.

tamird · 2016-12-20T20:02:41Z

Probably fine as-is, then.

gmlewis · 2016-12-21T17:57:35Z

Yes, I'm fine with both options. It's interesting how often we end up writing code at work that jumps back and forth between []byte and string and I remember hearing some famous Gophers from the Go team lament this fact.

This change makes the test names more consistent. Updates #481.

Closes google#481. Updates google#6.

This change makes the test names more consistent. Updates google#481.

Follows the same high-level pattern as set out in PR #481 with PullRequestsService.GetRaw method. Improve documentation wording. Improve style of PullRequestsService.GetRaw and its tests to be more consistent. Fix minor typo in GitHub documentation URL.

Follows the same high-level pattern as set out in PR #481 with PullRequestsService.GetRaw method. Improve documentation wording. Improve style of PullRequestsService.GetRaw and its tests to be more consistent. Fix minor typo in GitHub documentation URL (2 slashes instead of 1).

Follows the same high-level pattern as set out in PR google#481 with PullRequestsService.GetRaw method. Improve documentation wording. Improve style of PullRequestsService.GetRaw and its tests to be more consistent. Fix minor typo in GitHub documentation URL (2 slashes instead of 1).

Follows the same high-level pattern as set out in PR google#481 with PullRequestsService.GetRaw method.

huydx force-pushed the get-raw-patch branch from 46dfd8d to 87673ef Compare December 2, 2016 05:20

huydx force-pushed the get-raw-patch branch from 87673ef to 63bef7b Compare December 2, 2016 06:11

dmitshur reviewed Dec 2, 2016

View reviewed changes

huydx force-pushed the get-raw-patch branch 2 times, most recently from 6dcc781 to ce4ce33 Compare December 13, 2016 13:40

dmitshur reviewed Dec 13, 2016

View reviewed changes

huydx force-pushed the get-raw-patch branch 3 times, most recently from 9180d93 to 4a9e2b1 Compare December 14, 2016 01:29

dmitshur reviewed Dec 14, 2016

View reviewed changes

gmlewis requested changes Dec 14, 2016

View reviewed changes

huydx force-pushed the get-raw-patch branch from 4a9e2b1 to 9835668 Compare December 14, 2016 10:16

Add function for PullRequestService to download raw content of pull r…

8c92f7c

…equest

huydx force-pushed the get-raw-patch branch from 491be06 to 8c92f7c Compare December 19, 2016 14:53

dmitshur approved these changes Dec 19, 2016

View reviewed changes

dmitshur reviewed Dec 19, 2016

View reviewed changes

dmitshur closed this in 7edde48 Dec 19, 2016

dmitshur added a commit that referenced this pull request Dec 22, 2016

Rename 3 TestPullRequestService_ tests for consistency.

64431e1

This change makes the test names more consistent. Updates #481.

dmitshur mentioned this pull request Feb 8, 2017

handling custom media types #6

Closed

bubg-dev pushed a commit to bubg-dev/go-github that referenced this pull request Jun 16, 2017

Add PullRequests.GetRaw to download raw content of pull request.

941585a

Closes google#481. Updates google#6.

bubg-dev pushed a commit to bubg-dev/go-github that referenced this pull request Jun 16, 2017

Rename 3 TestPullRequestService_ tests for consistency.

8f91916

This change makes the test names more consistent. Updates google#481.

dmitshur mentioned this pull request Jun 20, 2017

Raw GetBlob() support #658

Closed

dmitshur mentioned this pull request Sep 26, 2017

How to render readme content as HTML? #727

Closed

dmitshur mentioned this pull request Oct 29, 2017

Add RepositoriesService.GetCommitRaw method. #767

Merged

dmitshur mentioned this pull request Nov 5, 2017

Provide capability to specify the Accept request header when retrieving issues. #545

Closed

dmitshur mentioned this pull request Apr 11, 2018

Add GitService.GetBlobRaw. #891

Merged

bradleymcallister97 added a commit to bradleymcallister97/go-github that referenced this pull request Jul 21, 2020

Add RepositoriesService.CompareCommitsRaw method.

feab181

Follows the same high-level pattern as set out in PR google#481 with PullRequestsService.GetRaw method.

bradleymcallister97 mentioned this pull request Jul 21, 2020

Add RepositoriesService.CompareCommitsRaw method #1582

Merged

Add function for PullRequestService to download raw content of pull r… #481

Add function for PullRequestService to download raw content of pull r… #481

Conversation

huydx commented Dec 2, 2016

googlebot commented Dec 2, 2016

googlebot commented Dec 2, 2016

huydx commented Dec 2, 2016

gmlewis commented Dec 2, 2016

huydx commented Dec 2, 2016

dmitshur left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huydx commented Dec 3, 2016 • edited Loading

dmitshur commented Dec 4, 2016 • edited Loading

huydx commented Dec 6, 2016

dmitshur commented Dec 6, 2016

gmlewis commented Dec 6, 2016

huydx commented Dec 13, 2016

dmitshur commented Dec 13, 2016

huydx commented Dec 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitshur Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitshur Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huydx commented Dec 14, 2016 • edited Loading

Choose a reason for hiding this comment

dmitshur Dec 14, 2016 • edited Loading

Choose a reason for hiding this comment

dmitshur commented Dec 14, 2016

gmlewis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huydx commented Dec 14, 2016

huydx commented Dec 19, 2016

dmitshur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitshur commented Dec 19, 2016 • edited Loading

huydx commented Dec 20, 2016

tamird commented Dec 20, 2016 • edited Loading

dmitshur commented Dec 20, 2016

tamird commented Dec 20, 2016 via email

dmitshur commented Dec 20, 2016 • edited Loading

tamird commented Dec 20, 2016

dmitshur commented Dec 20, 2016 • edited Loading

tamird commented Dec 20, 2016

gmlewis commented Dec 21, 2016

dmitshur left a comment •

edited

Loading

huydx commented Dec 3, 2016 •

edited

Loading

dmitshur commented Dec 4, 2016 •

edited

Loading

dmitshur Dec 13, 2016 •

edited

Loading

dmitshur Dec 13, 2016 •

edited

Loading

huydx commented Dec 14, 2016 •

edited

Loading

dmitshur Dec 14, 2016 •

edited

Loading

dmitshur commented Dec 19, 2016 •

edited

Loading

tamird commented Dec 20, 2016 •

edited

Loading

dmitshur commented Dec 20, 2016 •

edited

Loading

dmitshur commented Dec 20, 2016 •

edited

Loading