Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
egorse opened this issue Mar 7, 2018 · 4 comments
Closed

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

egorse opened this issue Mar 7, 2018 · 4 comments

Comments

@egorse
Copy link
Contributor

egorse commented Mar 7, 2018

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
Copy link
Collaborator

dmitshur commented Mar 7, 2018

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
Copy link
Contributor Author

egorse commented Mar 7, 2018

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

@dmitshur
Copy link
Collaborator

dmitshur commented Mar 7, 2018

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
Copy link
Contributor Author

egorse commented Mar 7, 2018

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

andygrunwald added a commit that referenced this issue Mar 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants