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

chore: demote sinon to dev dependencies #202

Merged
merged 3 commits into from Aug 15, 2018
Merged

chore: demote sinon to dev dependencies #202

merged 3 commits into from Aug 15, 2018

Conversation

vkarpov15
Copy link
Contributor

Sinon doesn't need to be a prod dependency, the only place you use it is here: https://github.com/intercom/intercom-node/blob/4bd04979a0a0ba962d0f396fba9bf4c39f2ebefd/test/user-data.js

vkarpov15 referenced this pull request in sinonjs/sinon May 10, 2018
We have managed to pay back outstanding expenses
@jpike88
Copy link

jpike88 commented Jul 26, 2018

I really would prefer installation without this donation message too.

screen shot 2018-07-26 at 1 59 48 pm

sinonjs/sinon#1737

@jpike88
Copy link

jpike88 commented Aug 8, 2018

@jonnyom this is such a minor, fast pr to accept. Can you please just let it through?

@jpike88
Copy link

jpike88 commented Aug 8, 2018

@vkarpov15 actually could you amend this PR to an updated version of sinon? They deleted that annoying as hell donation message, it's incredibly grating to see that love heart appearing in all my build logs.

@jonnyom
Copy link
Contributor

jonnyom commented Aug 8, 2018

Hey @jpike88 and @vkarpov15 thanks for this.

Happy to approve this, I can't see any reason for that to be in the production dependencies.

@vkarpov15 as Josh mentioned, could you please update to the latest version of Sinon? Be happy to approve then.

Thanks a million 👍

@vkarpov15
Copy link
Contributor Author

@jonnyom done.

@jpike88
Copy link

jpike88 commented Aug 9, 2018

@jonnyom just confirming that the donation message issue is fixed

Copy link
Contributor

@jonnyom jonnyom left a comment

Choose a reason for hiding this comment

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

LGTM. 👍 will look into getting a release set up for this ASAP

@jpike88
Copy link

jpike88 commented Aug 14, 2018

@jonnyom whats our ETA on approving this? I'm getting triggered constantly in my build logs

@jonnyom
Copy link
Contributor

jonnyom commented Aug 15, 2018

@jpike88 - we identified an issue with CI not building forked repos, which is where the delay came from here. We should have this updated and released to a new version by next week.

Thanks for the patience on this one.

@jonnyom jonnyom merged commit c3a59fa into intercom:master Aug 15, 2018
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.

None yet

3 participants