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

WIP: [EXAMPLE] Convert the project to yarn/NPM and add linting/testing #11

Closed
wants to merge 2 commits into from

Conversation

rhbvkleef
Copy link
Contributor

No description provided.

@shiffman
Copy link
Member

shiffman commented Mar 9, 2018

Thank you for this! I'm going to hold of on adding this later as it is not presently the focus of the tutorial. Referencing #3.

@shiffman
Copy link
Member

I think I'm maybe not ready for webpack and bundling yet? I guess I should probably make a video tutorial about this? Adding the linting is great though!

@rhbvkleef
Copy link
Contributor Author

Let me know if you want me to restructure this and remove webpack. I'll close this since we won't merge this.

@rhbvkleef rhbvkleef closed this Mar 10, 2018
@shiffman
Copy link
Member

Yes, I'm happy to merge a pull request that includes linting without webpack! Unit tests would also be welcome and I'd love to setup a CI for the linting and testing. (Unless all of this is a bad idea for some other reason?)

@rhbvkleef
Copy link
Contributor Author

The current code structure is actually very bad for unit testing. A way around it would be to mock P5 functions. We'd need to restructure it quite a lot if we don't want to mock. I'll create a merge request for the rest momentarily.

@shiffman
Copy link
Member

Great, yes, let's leave out unit testing for now.

@rhbvkleef rhbvkleef deleted the 3-add-linting branch March 25, 2018 12:57
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.

2 participants