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

Jump to bottom

Limit export node default assignment stack size #4323 #5235

Merged
merged 1 commit into from Jan 28, 2017
Merged

Limit export node default assignment stack size #4323 #5235

merged 1 commit into from Jan 28, 2017

Conversation

mattste
Copy link
Contributor

@mattste mattste commented Jan 28, 2017

Signed-off-by: Matthew Stewart mattdste@gmail.com

Q A
Patch: Bug Fix? Yes
Major: Breaking Change? No
Minor: New Feature? No
Deprecations? No
Spec Compliancy?
Tests Added/Pass? Yes
Fixed Tickets Fixes #4323
License MIT
Doc PR No
Dependency Changes No

@mattste
Limit export node default assignment stack size babel#4323
d479088 
Signed-off-by: Matthew Stewart <mattdste@gmail.com>
@mention-bot
Copy link

@mattste, thanks for your PR! By analyzing the history of the files in this pull request, we identified @hzoo, @loganfsmyth and @benjamn to be potential reviewers.

@mattste
Copy link
Contributor Author

mattste commented Jan 28, 2017 •
edited

It's my first P/R, so sorry if something is amiss. I'll gladly make any necessary changes. I'm on the Slack w/ handle @mattste for any questions

The biggest issue I am curious about right now is the test case. It's larger than all of the ones currently in the repo because we decided to limit the assignment length to 100.

@codecov-io
Copy link

codecov-io commented Jan 28, 2017 •
edited

Current coverage is 89.22% (diff: 100%)

No coverage report found for master at a97be35.

Powered by Codecov. Last update a97be35...d479088

@hzoo hzoo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Jan 28, 2017
loganfsmyth
loganfsmyth approved these changes Jan 28, 2017
View reviewed changes
Copy link
Member

@loganfsmyth loganfsmyth left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@loganfsmyth loganfsmyth merged commit 510e93b into babel:master Jan 28, 2017
@hzoo
Copy link
Member

hzoo commented Jan 28, 2017

It's larger than all of the ones currently in the repo because we decided to limit the assignment length to 100.

Sounds like it was necessary to test the logic correctly though unless we mocked the starting number so 👍

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@loganfsmyth loganfsmyth loganfsmyth approved these changes

Assignees
No one assigned
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Maximum call stack size exceeded when building lib via babel CLI (T7402)
5 participants
@mattste @mention-bot @codecov-io @hzoo @loganfsmyth