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

Chat Example Project #16

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Chat Example Project #16

wants to merge 4 commits into from

Conversation

cryptoquick
Copy link
Collaborator

@cryptoquick cryptoquick commented Jul 29, 2020

This PR is a small example project of a chat app written with jszmq. I decided to do this work first before the async/await work in #4 because this work will help a great deal with the conversion.

Tasks

  • Chat Example

@cryptoquick
Copy link
Collaborator Author

So, Cypress has a limitation that prevents it from being used to test across multiple windows. It recommends stubbing that behavior instead, which isn't really a great end to end test. It makes a lot of sense to do that, but not with this example project. I'm going to reduce the scope of this pull request for now.

The rest of the work is finished, however, so that could prove useful to others seeking to use the library in its current state, and it'll also be very useful for testing when moving towards an async/await API.

@cryptoquick cryptoquick changed the title [WIP] Chat Example Project & Cypress Integration Test [WIP] Chat Example Project Aug 1, 2020
@cryptoquick cryptoquick marked this pull request as ready for review August 1, 2020 21:04
@cryptoquick cryptoquick changed the title [WIP] Chat Example Project Chat Example Project Aug 1, 2020
@cryptoquick
Copy link
Collaborator Author

One more thing... I just realized, I forgot to exclude the example project from the jszmq example config when tests are compiled. I had split them up into two separate configs as an optimization step. At this point I forget why... But I'll add the example node_modules to the clear:all script, too.

@cryptoquick
Copy link
Collaborator Author

That should be RFR now. 👍

@cryptoquick
Copy link
Collaborator Author

I think #17 should be merged before this one.

The package version on this branch is still 0.1.3, but I think it would be wise to bump the semver minor on this one, to version 0.2.0.

@cryptoquick
Copy link
Collaborator Author

@somdoron Can you review #17 and #16 sometime? It's been a little while, lol!

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.

1 participant