Skip to content

Conversation

@WyriHaximus
Copy link
Member

No description provided.

@WyriHaximus WyriHaximus added this to the v0.8.0 milestone Oct 13, 2017
@WyriHaximus WyriHaximus requested review from clue and jsor October 13, 2017 06:12
@WyriHaximus WyriHaximus force-pushed the middleware-runner-correction-tests branch from 2837578 to d648444 Compare October 14, 2017 23:01
}
}

public function testCorruptMiddlewareStack()
Copy link
Member

Choose a reason for hiding this comment

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

The method name seems to imply something is "corrupt" here, can you update this to emphasize what is being tested here? :-) Otherwise LGTM 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

@clue fixed 👍

@WyriHaximus
Copy link
Member Author

Will rebase it once approved

}
}

public function testNextCanBeRunMoreThenOnceWithoutCorruptingTheMiddlewareStack()
Copy link
Member

Choose a reason for hiding this comment

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

Then -> Than

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed 👍

@clue clue changed the title Test for middleware stack corruption by the middleware runner Test middleware runner to ensure next handler can be called more than once Nov 14, 2017
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

LGTM, please squash and let's get this in! :shipit:

@WyriHaximus WyriHaximus force-pushed the middleware-runner-correction-tests branch 2 times, most recently from 958a1e8 to fde33f7 Compare November 14, 2017 16:28
@WyriHaximus
Copy link
Member Author

Err whoops I think I accedently rebased my commit away 0_O. Will file a new PR -.-

@WyriHaximus
Copy link
Member Author

Fixed the branch thanks to @jsor as mentioned in #245 (comment)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants