Skip to content

refactor to simplify handling json streams#13754

Draft
thaJeztah wants to merge 3 commits intodocker:mainfrom
thaJeztah:simplify_jsonstream
Draft

refactor to simplify handling json streams#13754
thaJeztah wants to merge 3 commits intodocker:mainfrom
thaJeztah:simplify_jsonstream

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

What I did

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Removes the need to manually convert to a string.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
}
}

err = jsonmessage.DisplayJSONMessagesStream(response.Body, buildBuff, progBuff.FD(), true, aux)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this one works; the old code was unconditionally setting "terminal" to "true".

Comment thread pkg/bridge/convert.go
Comment on lines -181 to 184
exposed.Add(strconv.Itoa(int(p.Num())))
exposed.Add(p.Port())
}
for _, port := range service.Ports {
exposed.Add(strconv.Itoa(int(port.Target)))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not introduced in my changes, but wondering if this code is correct, or if it should preserve the protocol (tcp, udp, ...) https://docs.docker.com/reference/compose-file/services/#expose

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/bridge/convert.go 0.00% 1 Missing and 1 partial ⚠️
pkg/compose/build_classic.go 33.33% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

1 participant