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

Fix recursion problem more generally #3989

Merged
merged 3 commits into from Apr 20, 2018

Conversation

Projects
None yet
2 participants
@agjohnson
Contributor

agjohnson commented Apr 20, 2018

Remove previous fixes that were trying to catch specific cases, apply a general
solution that tests if we're in a loop.

Fix recursion problem more generally
Remove previous fixes that were trying to catch specific cases, apply a general
solution that tests if we're in a loop.

@agjohnson agjohnson requested a review from rtfd/core Apr 20, 2018

@agjohnson agjohnson added this to the 2.4 milestone Apr 20, 2018

# recursion. We can't determine a root project well here, so you get
# what you get if you have configured your project in a strange manner
if projects is None:
projects = [project]

This comment has been minimized.

@davidfischer

davidfischer Apr 20, 2018

Contributor

Don't you want an else here that does projects.append(project)?

This comment has been minimized.

@agjohnson

agjohnson Apr 20, 2018

Contributor

oops, yes, that is the missing piece

@davidfischer

I just reviewed visually but I believe we are good.

@agjohnson agjohnson removed the PR: hotfix label Apr 20, 2018

@agjohnson

This comment has been minimized.

Contributor

agjohnson commented Apr 20, 2018

I added one last test to ensure deeper nesting was tested, no logic changed though

@agjohnson agjohnson merged commit 9fcf878 into master Apr 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@agjohnson agjohnson deleted the agj/another-recursion-fix branch Apr 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment