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

Issue #199: Put the closure in heap to avoid the stack overflowed in Debug #200

Closed
wants to merge 7 commits into from

Conversation

streetycat
Copy link
Collaborator

No description provided.

@lurenpluto lurenpluto self-requested a review May 5, 2023 04:15
Copy link
Member

@lurenpluto lurenpluto left a comment

Choose a reason for hiding this comment

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

What are the key changes to this fix? It looks like it's all unrelated changes

for e in self {

// stable sort
let mut values: Vec<&T> = self.iter().collect();
Copy link
Member

Choose a reason for hiding this comment

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

This should be the change mentioned by #256, it should not be in this pull request

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sort the values in ascending order, and encode the elements into the buffer. for Issue #256

Copy link
Member

Choose a reason for hiding this comment

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

I mean it should be included in a new request for issue #256, and in the entire pull request, it seems, this is the only substantive change?

Copy link
Member

Choose a reason for hiding this comment

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

This file only seems to have changed in format, and I compared it to see no substantive changes, and the changes seem to have nothing to do with the problem the target is trying to fix

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, It's only format change. It occurs when I merged the main branch to my local branch, and the IDE changed the format automatically, I think we shoud define the standuard configure of format for IDE.

@lurenpluto lurenpluto self-requested a review May 10, 2023 11:50
Copy link
Member

@lurenpluto lurenpluto left a comment

Choose a reason for hiding this comment

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

From the log of your pull request's commits, the whole operation list is a bit complicated and confused, you should make the relevant changes for each issue frok a separate branch and then create the pull request, not mix it on a larger functional branch and bring up part of the commit inside as a pull request

@streetycat streetycat closed this May 12, 2023
@streetycat streetycat deleted the buckyos branch May 12, 2023 07:15
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.

4 participants