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 "proxyquire" dependency #6797

Merged
merged 9 commits into from Feb 21, 2017
Merged

Remove "proxyquire" dependency #6797

merged 9 commits into from Feb 21, 2017

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Feb 21, 2017

proxyquire is hard to use and understand and we can do much better with a much simpler API in many cases

return mergeTreesStub.apply(this, arguments);
},
});
let mergeTrees = require('../../../lib/broccoli/merge-trees')
Copy link
Member

Choose a reason for hiding this comment

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

missing semi-colon, also we generally use const for required things...

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops... fixed!

@homu
Copy link
Contributor

homu commented Feb 21, 2017

☔ The latest upstream changes (presumably #6729) made this pull request unmergeable. Please resolve the merge conflicts.

@Turbo87
Copy link
Member Author

Turbo87 commented Feb 21, 2017

@rwjblue fixed the failing tests. should be ready now.

@kellyselden
Copy link
Member

The addition of underscored member functions is what I was trying to avoid when adding this. That said, I'm not necessarily against this.

@Turbo87
Copy link
Member Author

Turbo87 commented Feb 21, 2017

@homu r+

@homu
Copy link
Contributor

homu commented Feb 21, 2017

📌 Commit fbb5985 has been approved by Turbo87

homu added a commit that referenced this pull request Feb 21, 2017
Remove "proxyquire" dependency

`proxyquire` is hard to use and understand and we can do much better with a much simpler API in many cases
@homu
Copy link
Contributor

homu commented Feb 21, 2017

⌛ Testing commit fbb5985 with merge 21a2e44...

@homu
Copy link
Contributor

homu commented Feb 21, 2017

☀️ Test successful - status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants