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.

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

fix: omit empty pins slice when reporting pin progress #8023

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

Stebalien
Copy link
Member

Otherwise, we get messages of the form {Pins: null, Progress: 100}. After this change, we'll get {Progress: 100}.

Otherwise, we get messages of the form `{Pins: null, Progress: 100}`.
After this change, we'll get `{Progress: 100}`.
@Stebalien
Copy link
Member Author

@achingbrain does this change have any impact on the HTTP API as far as js-ipfs is concerned?

@Stebalien
Copy link
Member Author

Ping @achingbrain?

@achingbrain
Copy link
Member

No, js-IPFS doesn't report pinning progress yet, I was waiting for #7938 to determine what sort of behaviour should be implemented.

Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

Left a question, but since it works for @achingbrain in js-ipfs LGTM

@@ -44,8 +44,8 @@ type PinOutput struct {
}

type AddPinOutput struct {
Pins []string
Progress int `json:",omitempty"`
Pins []string `json:",omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the idea here that while the progress is being reported there's no reason to report the empty pins?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Otherwise, it could look like nothing was pinned.

@Stebalien Stebalien merged commit c1a0eb5 into master Apr 28, 2021
@aschmahmann aschmahmann mentioned this pull request May 14, 2021
71 tasks
@hacdias hacdias deleted the fix/pins-null branch May 9, 2023 11:01
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.

3 participants