search

andygrunwald / go-gerrit

Go client/library for Gerrit Code Review
https://godoc.org/github.com/andygrunwald/go-gerrit
MIT License
96 stars 40 forks source link

DiffContent.A/B/AB should be []string instead of string #44

Closed egorse closed 6 years ago

egorse commented 6 years ago

As per https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#get-diff (and per run againts Gerrit 2.13.7) seems the DiffContent.A/B/AB should be []string.

The reason I don`t make the PR is - I am not sure was that always broken or its just changed in some newer Gerrit version

dmitshur commented 6 years ago

I believe so. I made some changes in my local copy to make things work (but haven’t had the chance to try to upstream these changes yet), and I have the following:

// DiffContent entity contains information about the content differences in a file.
type DiffContent struct {
    A      []string          `json:"a,omitempty"`
    B      []string          `json:"b,omitempty"`
    AB     []string          `json:"ab,omitempty"`
    EditA  DiffIntralineInfo `json:"edit_a,omitempty"`
    EditB  DiffIntralineInfo `json:"edit_b,omitempty"`
    Skip   int               `json:"skip,omitempty"`
    Common bool              `json:"common,omitempty"`
}
egorse commented 6 years ago

@shurcooL worth mentioning the DiffIntralineInfo should be [][2]int - but as well I am not sure that's is not Gerrit version dependand :roll_eyes:

dmitshur commented 6 years ago

I see. I haven't needed EditA/EditB fields yet, that's why I haven't touched them.

The README specifies which version of Gerrit API this library targets. It is 2.11.3-1230-gb8336f1 at this time. It should be possible to confirm whether that version needs this change.

egorse commented 6 years ago

As per https://gerrit.googlesource.com/gerrit/+/v2.11/Documentation/rest-api-changes.txt#3068 seems its a bug - for A/B/AB and EditA/EditB