Skip to content

Add the return value for the set_next_stack_limit function referenced…#5433

Closed
spencer-burke wants to merge 1 commit into
adafruit:mainfrom
spencer-burke:main
Closed

Add the return value for the set_next_stack_limit function referenced…#5433
spencer-burke wants to merge 1 commit into
adafruit:mainfrom
spencer-burke:main

Conversation

@spencer-burke
Copy link
Copy Markdown

… in #4520

Changes to be committed:
modified: shared-bindings/supervisor/init.c

… in adafruit#4520

Changes to be committed:
	modified:   shared-bindings/supervisor/__init__.c
@spencer-burke
Copy link
Copy Markdown
Author

I have made a change and created the pr, if you would like me to write some tests of some kind I would be happy to.

Copy link
Copy Markdown
Collaborator

@microdev1 microdev1 left a 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 @spencer-burke.

I don't think this resolves issue #4520. The stack size is set on the next VM run so with the function call there isn't a way to confirm whether the requested stack_limit is used or the default_limit is used.

I think changing the function name to something more appropriate like set_next_stack_limit_maybe should be enough here. This will result in a breaking change and the PR will have to be considered for 8.0.0 rather than merging it now

@spencer-burke
Copy link
Copy Markdown
Author

Alright, I will revert the changes I have made, and change the function name.

@tannewt
Copy link
Copy Markdown
Member

tannewt commented Oct 7, 2021

I don't think it's helpful to return the passed in value. Thanks for the PR anyway!

@tannewt tannewt closed this Oct 7, 2021
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