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

(#326) - Update install and fixed polling problem #329

Closed
wants to merge 1 commit into from

Conversation

yaronyg
Copy link
Member

@yaronyg yaronyg commented Jun 4, 2016

#326

.gitignore - A bunch of files it would be good not to check in :)

test-setup.sh - To adapt to the new way of building PouchDB I now clone
PouchDB into its own separate directory, pouchdbclone and then do an
install and build, symbolically link to packages/pouchdb and install that
as well.

test-pouchdb-minimum.sh - The tests (as opposed to the live code) now
lives in pouchdbclone

test-pouchdb.sh - Same as previous

changes - As explained in #326 I tried to hook all the obvious events and
literally nothing fired when a connection was lost. So instead when
the heartbeat runs I check if we have lost the socket and if so then
I clean up. I also made cleanup able to cancel any live changes objects
and call res.end.

.gitignore - A bunch of files it would be good not to check in :)

test-setup.sh - To adapt to the new way of building PouchDB I now clone
PouchDB into its own separate directory, pouchdbclone and then do an
install and build, symbolically link to packages/pouchdb and install that
as well.

test-pouchdb-minimum.sh - The tests (as opposed to the live code) now
lives in pouchdbclone

test-pouchdb.sh - Same as previous

changes - As explained in pouchdb#326 I tried to hook all the obvious events and
literally nothing fired when a connection was lost. So instead when
the heartbeat runs I check if we have lost the socket and if so then
I clean up. I also made cleanup able to cancel any live changes objects
and call res.end.
@yaronyg
Copy link
Member Author

yaronyg commented Jun 4, 2016

I'm tracking Travis but I actually expect this to fail because of #328. But I have spent all day on this and I need to get back to coding my day job. So if it fails as expected then I'll have to see if I can find time to fix. But as I noted in #328 the failure was always there, this checkin didn't create. It's just that we didn't re-run the tests after putting in the hook to detect unhandled promise rejections.

@yaronyg
Copy link
Member Author

yaronyg commented Jun 4, 2016

Looks like i spoke way too soon. It's dying for a completely different reason. But what the heck did I possibly do to break jshint?

@garrensmith
Copy link
Member

@yaronyg you need to lock jshint down to 2.8.0. They have a breaking change once they are above 2.9

@yaronyg
Copy link
Member Author

yaronyg commented Jun 8, 2016

@marten-de-vries, above @garrensmith makes the point that we shouldn't have JSHint above 2.8.0 because of breaking changes. I checked and you were the one who approved Greenkeeper bumping us up. I wanted to therefore check with you first that there isn't some specific reason why we went above 2.8.0? How did you get tests to pass?

@yaronyg
Copy link
Member Author

yaronyg commented Jun 8, 2016

Just leaving myself a note that I need to update this PR based on the work @garrensmith does in pouchdb/pouchdb-server#140

@marten-de-vries
Copy link
Member

@yaronyg Something weird is going on with the test setup then: https://travis-ci.org/pouchdb/express-pouchdb/jobs/124334134#L263 was green. Maybe it used a different jshint install (it definitely installed the new one, as you can see on the highlighted line), although I wouldn't know where it could find another one.

Anyway, I approved that just as a routine update, so feel free to revert it if it's causing trouble now.

@yaronyg
Copy link
Member Author

yaronyg commented Jun 8, 2016

@marten-de-vries O.k. I'll experiment and see if changing the version fixes things.

@garrensmith
Copy link
Member

I've got a PR #342 that should fix all these issues.

@garrensmith
Copy link
Member

@yaronyg could you either close this PR and open another one with the changes work or adjust this one to just be the changes fixes.

@yaronyg
Copy link
Member Author

yaronyg commented Jul 5, 2016

@garrensmith Makes sense. I'm going to keep this open for now so I can track and then close when I have the new PR ready.

@yaronyg
Copy link
Member Author

yaronyg commented Jul 8, 2016

In the midst of preparing the new PR using the pouchdb-server monorepo.

@yaronyg yaronyg closed this Jul 8, 2016
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