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

Remove unused chain behavior #1326

Merged
merged 1 commit into from
Mar 12, 2017

Conversation

lucasfcosta
Copy link
Member

Purpose (TL;DR)

This PR removes the old chain behavior method introduced in #1227 since we're not using it anymore.

This is also related to #817 and #1100.

Background (Problem in detail)

Since #1302, most of the old behaviors were moved to a file called defaultBehaviors and now we're using the addBehavior method to add behavior methods to the stub. This method assigns a new function to the stub's proto and this method both runs the behavior method and then returns this.stub or this if this.stub is falsy, which is exactly the same thing the chain behavior did.

Due to this fact, the chain method is not useful anymore because we're not using it internally anymore and no user needs to call it, since every behavior will return exactly the same value (the stub itself). Another point that backs up this assumption is that we have no tests that test chain specifically, which means this isn't something meant to be used by our end-users.

Solution

Given the explanation above, I simply removed the chain behavior and all tests still pass.

How to verify

  1. Check out this branch (see github instructions below)
  2. npm install
  3. npm test -> All tests still pass

Let me know if I missed anything and I'll be more than happy to fix it. Thanks!

@lucasfcosta lucasfcosta force-pushed the remove-unused-chain-behavior branch from 134c1ee to 70e96f5 Compare March 12, 2017 16:51
@coveralls
Copy link

coveralls commented Mar 12, 2017

Coverage Status

Coverage decreased (-1.4%) to 93.984% when pulling 70e96f5 on lucasfcosta:remove-unused-chain-behavior into e0cfccc on sinonjs:master.

@coveralls
Copy link

coveralls commented Mar 12, 2017

Coverage Status

Coverage decreased (-1.4%) to 93.984% when pulling 70e96f5 on lucasfcosta:remove-unused-chain-behavior into e0cfccc on sinonjs:master.

@mantoni
Copy link
Member

mantoni commented Mar 12, 2017

Given the simplicity of this change, I think it could go into the still pending 2.0 release @fatso83 @mroderick @cjohansen?

@fatso83
Copy link
Contributor

fatso83 commented Mar 12, 2017

Absolutely, I deleted that code myself in another (unmerged) branch. Merging.

@fatso83 fatso83 merged commit 1bda7c4 into sinonjs:master Mar 12, 2017
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