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.

Sign up for GitHub

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

Jump to bottom

Swap out json-iterator for golang default #8378

Merged
merged 1 commit into from
Nov 18, 2020
Merged

Swap out json-iterator for golang default #8378

merged 1 commit into from
Nov 18, 2020

Conversation

jwhonce
Copy link
Member

@jwhonce jwhonce commented Nov 17, 2020

json-iterator library failed to pretty print json for all inspection
types.

Fixes #8366

Signed-off-by: Jhon Honce [email protected]

@jwhonce
Swap out json-iterator for golang default
4ed1ef5 
json-iterator library failed to pretty print json for all inspection
types.

Fixes containers#8366

Signed-off-by: Jhon Honce <[email protected]>
@jwhonce jwhonce added the kind/bug Categorizes issue or PR as related to a bug. label Nov 17, 2020
@jwhonce jwhonce requested a review from ashley-cui November 17, 2020 18:46
@jwhonce jwhonce self-assigned this Nov 17, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jwhonce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 17, 2020
@rhatdan
Copy link
Member

rhatdan commented Nov 17, 2020

LGTM

@edsantiago
Copy link
Member

jhonce, I assume it's too late to request tests. Can you offer me suggestions on how I can confirm this, including the erroneous behavior prior to this fix?

@jwhonce
Copy link
Member Author

jwhonce commented Nov 17, 2020 •
edited
Loading

@edsantiago I didn't add tests as they would be brittle. Previous behavior the output was randomly indented when doing podman network inspect ... see #8366. Now each embedded stanza is indented as expected. Easy visual check, very hard to do programmatically as any new field or stanza in output could change indentation.

@mheon
Copy link
Member

mheon commented Nov 17, 2020

LGTM but I'll defer merge until the tests discussion is settled

@rhatdan
Copy link
Member

rhatdan commented Nov 18, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2020
@openshift-merge-robot openshift-merge-robot merged commit 4bbf2b6 into containers:master Nov 18, 2020
@jwhonce jwhonce deleted the issues/8366 branch June 30, 2021 16:12
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ashley-cui ashley-cui Awaiting requested review from ashley-cui

Assignees

@jwhonce jwhonce

@rhatdan rhatdan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: podman network inspect wrong ident
6 participants
@jwhonce @openshift-ci-robot @rhatdan @edsantiago @mheon @openshift-merge-robot