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

Pinning interop: Pin ls returns appropriate zero value #6685

Merged
merged 2 commits into from
Oct 7, 2019
Merged

Pinning interop: Pin ls returns appropriate zero value #6685

merged 2 commits into from
Oct 7, 2019

Conversation

aschmahmann
Copy link
Contributor

No description provided.

@aschmahmann aschmahmann changed the title Pin ls indirect pins even if parents are directly pinned Pinning interop: Pin ls returns appropriate zero value Sep 27, 2019
@aschmahmann
Copy link
Contributor Author

aschmahmann commented Sep 27, 2019 •
edited
Loading

We don't have any streaming interop tests so I'm not sure if we should remove the json:",omitempty" from PinLsObject as well.

This may also be a bug on the part of the HTTP client, any thoughts?

@aschmahmann aschmahmann self-assigned this Sep 27, 2019
@Stebalien
Copy link
Member

Ugh. We want to omit PinLsList.Keys if we're streaming.

What if we set Keys to a non-nil empty map? That will cause us to send the correct {}.

@Stebalien
Copy link
Member

When not streaming, that is.

@Stebalien
Copy link
Member

Wait, that's what we're doing.

@Stebalien
Copy link
Member

Ah. I didn't test this the right way. omitempty doesn't care about nil versus non-nil empty.

Stebalien
Stebalien approved these changes Oct 7, 2019
View reviewed changes
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM. The test failures are unrelated.

@Stebalien Stebalien marked this pull request as ready for review October 7, 2019 23:42
@Stebalien Stebalien merged commit 14a0952 into ipfs:master Oct 7, 2019
@aschmahmann aschmahmann deleted the fix/6527 branch October 8, 2019 00:31
@aschmahmann aschmahmann mentioned this pull request Oct 8, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Stebalien Stebalien Stebalien approved these changes

Assignees

@aschmahmann aschmahmann

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aschmahmann @Stebalien