Skip to content

changes.go: fix json key #187

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

Closed
wants to merge 1 commit into from
Closed

changes.go: fix json key #187

wants to merge 1 commit into from

Conversation

flokli
Copy link

@flokli flokli commented May 15, 2025

At least the key in ChangeInfo is called wip, not work_in_progress:

{"uploader":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"patchSet":{"number":1,"revision":"ede307a009aa0b1eb62e9f18b7bf1f26e9fc98a9","parents":["9f8fb55318f2bafb37e4587fa4b6c793b2b540c0"],"ref":"refs/changes/25/30525/1","uploader":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"createdOn":1747335735,"author":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"kind":"REWORK","sizeInsertions":11,"sizeDeletions":1545},"change":{"project":"snix","branch":"canon","id":"If8faecdd018b45dd087b7332fe3d3a8280947358","number":30525,"subject":"fix(ops): drop clbot","owner":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"url":"https://cl.snix.dev/c/snix/+/30525","commitMessage":"fix(ops): drop clbot\n\nThis removes the old clbot, which kept an SSH connection to gerrit open.\n\nChange-Id: If8faecdd018b45dd087b7332fe3d3a8280947358\n","createdOn":1747335735,"status":"NEW","wip":true},"project":"snix","refName":"refs/heads/canon","changeKey":{"id":"If8faecdd018b45dd087b7332fe3d3a8280947358"},"type":"patchset-created","eventCreatedOn":1747335735}

(This is an example of what I received via gerrit stream-events)

At least the key in ChangeInfo is called `wip`, not `work_in_progress`:

```json
{"uploader":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"patchSet":{"number":1,"revision":"ede307a009aa0b1eb62e9f18b7bf1f26e9fc98a9","parents":["9f8fb55318f2bafb37e4587fa4b6c793b2b540c0"],"ref":"refs/changes/25/30525/1","uploader":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"createdOn":1747335735,"author":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"kind":"REWORK","sizeInsertions":11,"sizeDeletions":1545},"change":{"project":"snix","branch":"canon","id":"If8faecdd018b45dd087b7332fe3d3a8280947358","number":30525,"subject":"fix(ops): drop clbot","owner":{"name":"Florian Klink","email":"flokli@flokli.de","username":"flokli"},"url":"https://cl.snix.dev/c/snix/+/30525","commitMessage":"fix(ops): drop clbot\n\nThis removes the old clbot, which kept an SSH connection to gerrit open.\n\nChange-Id: If8faecdd018b45dd087b7332fe3d3a8280947358\n","createdOn":1747335735,"status":"NEW","wip":true},"project":"snix","refName":"refs/heads/canon","changeKey":{"id":"If8faecdd018b45dd087b7332fe3d3a8280947358"},"type":"patchset-created","eventCreatedOn":1747335735}
```
@andygrunwald
Copy link
Owner

@flokli Which version of Gerrit you are using?
The docs in https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-info say the key is names work_in_progress.

@flokli
Copy link
Author

flokli commented May 16, 2025

It's 3.11.0. I also noticed _number is called number there, not number.

I'm not sure the REST API looks exactly the same as what gerrit stream-events emits. Are there expected to be differences?

@andygrunwald
Copy link
Owner

See https://gerrit-review.googlesource.com/Documentation/cmd-stream-events.html#events and https://gerrit-review.googlesource.com/Documentation/json.html#change

It looks like these events follow a different structure than the REST API.
Right now, this library do not support the structures of Stream-Events (afaik).

I am happy to receive a PR towards this.

@flokli
Copy link
Author

flokli commented May 16, 2025

Ugh, this being separate (but similarly named) types is unfortunate.

How would you propose to handle this? Just add additional structs, named like in the documentation? Putting it in a separate (sub) package to avoid clashes?

@andygrunwald
Copy link
Owner

I don't know exactly, as i do not have experience with stream-events.
From the first view, a separate package stream or streamevents or similar sounds useful. This way, we can use the original names and avoid name conflicts.

This package can start with the data structures itself and later on, developed towards a proper client to use.

What do you think?

flokli added a commit to flokli/go-gerrit that referenced this pull request May 23, 2025
This contains definitions for all message types present in "JSON stream"
messages, produced by the `gerrit stream-events` command, or the webhook
plugin.

As per the discussion in
andygrunwald#187 (comment),
this is put in a separate package to avoid clashes with other types.

Closes andygrunwald#187.
@flokli
Copy link
Author

flokli commented May 24, 2025

Closing in favor of #189

@flokli flokli closed this May 24, 2025
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

Successfully merging this pull request may close these issues.

2 participants