-
Notifications
You must be signed in to change notification settings - Fork 315
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
optimize: copy_parents_data only needs base parents #1660
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! That looks correct to me. But I'd like to have other team members double-check as this is such a central piece.
@zhiqiangxu Thanks for your contributions. You also always put helpful context in your PR description. It would be cool if you could also put it into the commit message next time (then it even automatically becomes the PR description).
6070b0c
to
ca97f07
Compare
I just modified the commit message and re-pushed the branch :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Looks good to me.
copy_parents_data_inner only uses base parents, so there's no need to fetch extension parents at the caller side.
ca97f07
to
334a27b
Compare
@cryptonemo we should also merge that one. As you've most PR open, I'll leave it up to you when to merge. |
@zhiqiangxu sorry for taking so long, this PR slipped my attention. It's now merged. |
copy_parents_data_inner
only uses base parents, so there's no need to fetch extension parents at the caller side.