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

removed packages that weren't being used #112

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

dhuang612
Copy link
Contributor

So the build is failing according to github.

I opened an issue related to this and investigated
According to this:
microsoft/node-pty#319

It's something to do with node v 12 and windows builds.
They recommended installing node-pty@beta as a dev dependency which I did.
I'll review the travis logs to see if this resolves that.

The bundlesize command under package.json wasn't working.

updated bundlesize.config.json to accept module blobs

path": "dist/**/*.js",

tested and confirmed it can now review the bundle sizes of the dist folder.

removed lodash package from package.json
and added the two individual modules that are being used in this project
.isEmpty & .map

removed giphy package as it wasn't being used
giphy-api is being used, left as is.

removed electron-debug package as it wasn't being used.

switched .travis.yml back to use npm run lint since yarn doesn't seem to be configured with this project.
tested this by cloning a fresh project down to computer when you run yarn start

yarn: command not found

Let me know if there is anything else that needs to be changed!

@joshghent
Copy link
Owner

Looks good to me! Thanks a lot for your work! @dhuang612! 👍

Copy link
Collaborator

@jamescallumyoung jamescallumyoung left a comment

Choose a reason for hiding this comment

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

Looks fine to me. 👍

The gif-provider module provides it's own giphy-api dependency so we can remove that here.

@jamescallumyoung
Copy link
Collaborator

Hey @joshghent, this looks fine by me. I won't merge it until you approve it too though. Feel free to merge yourself if you approve 👍

@dhuang612 Thanks for the help here 👍 Good stuff.

@joshghent joshghent merged commit 48f1d23 into joshghent:master Oct 30, 2019
@joshghent
Copy link
Owner

@jamescallumyoung thanks for reviewing that mate 👍 Great work @dhuang612 🎉

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.

3 participants