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

Pinning fixes #4451

Merged
merged 5 commits into from
Dec 4, 2017
Merged

Pinning fixes #4451

merged 5 commits into from
Dec 4, 2017

Conversation

Stebalien
Copy link
Member

This came from an old commit that used a TODO context. Now that we have a real
context, use that.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
Otherwise, we could run a GC between adding and pinning.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
Now that we add nodes to the DAG when pinning, don't bother adding them twice.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
instead of resolving all the pins first and then pinning, pin after resolving
each pin.

This:

1. Avoids storing all the nodes in memory.
2. Avoids not showing pin progress.

fixes #4122

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@ghost ghost assigned Stebalien Dec 4, 2017
@ghost ghost added the status/in-progress In progress label Dec 4, 2017
@Stebalien Stebalien requested review from a user and Kubuxu December 4, 2017 03:07
@whyrusleeping whyrusleeping merged commit c7eddab into master Dec 4, 2017
@whyrusleeping whyrusleeping deleted the fix/better-pinning branch December 4, 2017 17:51
@ghost ghost removed the status/in-progress In progress label Dec 4, 2017
@Kubuxu
Copy link
Member

Kubuxu commented Dec 4, 2017

Hmm, I thought I sent out the CR (approve), looks like I did not.

Copy link
Member

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

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

LGTM

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