[WIP] dismantle pkg/jsonstream, pkg/streamformatter, pkg/progress#50564
[WIP] dismantle pkg/jsonstream, pkg/streamformatter, pkg/progress#50564thaJeztah wants to merge 9 commits intomoby:masterfrom
Conversation
| formatted = out.sf.formatStatus(prog.ID, prog.Message) | ||
| } else { | ||
| jsonProgress := jsonmessage.JSONProgress{Current: prog.Current, Total: prog.Total, HideCounts: prog.HideCounts, Units: prog.Units} | ||
| jsonProgress := jsonmessage.JSONProgress{ |
There was a problem hiding this comment.
streamformatter at a glance looks to be what's used to produce API responses in various places, but it looks to be needing the raw types from the API; currently it uses the jsonmessage package - need to look if we can unwrap that and use types from the API.
There was a problem hiding this comment.
Oh, and then it uses yet-another type; it uses progress.Progress from pkg/progress, which looks rather similar to the type I introduced in this PR, so perhaps that's the one we SHOULD be looking for to move to the API.
There was a problem hiding this comment.
Yeah I think with the current API shape I think it makes sense to include these in the API...
There was a problem hiding this comment.
Trying some bits (just pushed some hacky commits), but it's ... really messy, and not clear which type in pkg/ corresponds with which actual response (looking into some of those).
0b7300e to
182a347
Compare
| type jsonMessage struct { | ||
| Stream string `json:"stream,omitempty"` | ||
| Status string `json:"status,omitempty"` | ||
| Progress *jsonmessage.JSONProgress `json:"progressDetail,omitempty"` // TODO(thaJeztah): can this be a [jsonstream.Progress]? |
There was a problem hiding this comment.
I think that is fine, the jsonmessage library will decode it anyway. The unexported fields can't be used anyway, its only used internally in the jsonmessage package.
a3fcb71 to
59ddeb1
Compare
Also rename api type JSONError to Error Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Signed-off-by: Derek McGowan <derek@mcg.dev>
Move the type to the API, but embed it, so that we keep the methods on the struct in this package. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This package depends on jsonformatter.JSONProgress and jsonmessage.JSONMessage, and it looks like it requires some of those for their stringer interface. Signed-off-by: Derek McGowan <derek@mcg.dev> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Derek McGowan <derek@mcg.dev>
Signed-off-by: Derek McGowan <derek@mcg.dev> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Derek McGowan <derek@mcg.dev> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Derek McGowan <derek@mcg.dev> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Derek McGowan <derek@mcg.dev> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The API still returns it for backward-compatibility (but probably shouldn't), but we should no longer print it. This removes the fields, and their use. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
59ddeb1 to
334c134
Compare
This may take some time to fully untangle;
events.Messagetype- What I did
- How I did it
- How to verify it
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)