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

Pouchdb replicator #209

Merged
merged 21 commits into from
Feb 22, 2017
Merged

Pouchdb replicator #209

merged 21 commits into from
Feb 22, 2017

Conversation

garrensmith
Copy link
Member

This adds in the pouchdb-replicator plugin with full git history.

/cc @marten-de-vries

marten-de-vries and others added 19 commits June 30, 2014 20:54
… much untested). Also validation update (will become 1.1.0).
… some improvements to coroutines (nesting them, most notably)
…rotections for them like in CouchDB). No regressions, but JS test coverage isn't complete yet.
…3008300ce1dd58e9ff5a6c21322db548'

git-subtree-dir: packages/node_modules/pouchdb-replicator
git-subtree-mainline: 66fc789
git-subtree-split: 4008445
@garrensmith garrensmith mentioned this pull request Feb 20, 2017
12 tasks
"sanitize-filename": "^1.6.1",
"uuid": "^3.0.1",
"pouchdb-fauxton": "^0.0.5"
},
Copy link
Member

Choose a reason for hiding this comment

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

@garrensmith you actually don't need to add any dependencies in the sub-packages; just keep it empty. They'll automatically be filled in by release.sh.


#after_success:
# - npm run helper -- semantic-release

Copy link
Member

@nolanlawson nolanlawson Feb 20, 2017

Choose a reason for hiding this comment

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

we can remove this file assuming the top-level .travis.yml also runs the tests

@@ -0,0 +1,35 @@
{
"name": "pouchdb-replicator",
"version": "2.3.4",
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't need to include a version; that's automatically added at release time.

OTOH we have to make sure we can actually publish a new version that's the same verison as express-pouchdb/pouchdb-server. Since the latest pouchdb-replicator is 2.1.3 looks like we're good.

tests/mocha.opts Outdated
@@ -1,2 +1,3 @@
--colors
--compilers js:pouchdb-plugin-helper/mocha-babel
Copy link
Member

Choose a reason for hiding this comment

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

I would much prefer to keep things simple and not have any ES6+ in the codebase, but if that's how pouchdb-replicator and the other plugins are written, then we should probably have a unified strategy for what we're doing with transpilation. It seems ES6+ is currently only used in the tests, not the source code?

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively I would be okay with just converting from ES modules to CommonJS and using only Node 4+ features. We only support Node 4+ anyhow. Then we could get rid of the mocha-babel helper entirely.

Copy link
Member

Choose a reason for hiding this comment

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

The tests were originally written in Python. I ported them to ES6 since it allowed for the most direct translation. They depend quite heavily on async/await, which as far as I know is not supported even in node 4+.

Copy link
Member

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

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

Also we seem to be missing a LICENSE file?

"devDependencies": {
"pouchdb-plugin-helper": "^3.0.0"
},
"optionalDependencies": {}
Copy link
Member

Choose a reason for hiding this comment

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

We seem to be missing a LICENSE file for pouchdb-replicator and also "license": "Apache-2.0" in the package.json.

Copy link
Member

@marten-de-vries marten-de-vries Feb 20, 2017

Choose a reason for hiding this comment

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

Hmm, probably forgot that while moving to Github. Anyway, I always meant for this to be under Apache-2.0; re-licensing it under ISC or MIT license is fine with me too.

@nolanlawson
Copy link
Member

nolanlawson commented Feb 20, 2017

Thanks for putting the work in @garrensmith ! Just a few changes and it should be good to go. Also I added you as a publisher: https://www.npmjs.com/package/pouchdb-replicator

@garrensmith
Copy link
Member Author

garrensmith commented Feb 20, 2017 via email

@nolanlawson
Copy link
Member

nolanlawson commented Feb 20, 2017 via email

@marten-de-vries
Copy link
Member

Just commented inline:

Hmm, probably forgot that while moving to Github. Anyway, I always meant for this to be under Apache-2.0; re-licensing it under ISC or MIT license is fine with me too.

So feel free to do whatever is most practical/most in line with other packages. And nice to see all the monorepo work lately!

@garrensmith
Copy link
Member Author

I've converted all the tests to es2015. It took pretty long but I think its at least more consistent.
I've also made the other fixes.

@gr2m
Copy link
Contributor

gr2m commented Feb 21, 2017

awwman thanks for doing the hard work <3

@nolanlawson
Copy link
Member

@garrensmith looks like we still have dependencies/version in the sub-packages and a dangling .travis.yml file? Other than that I'm 👍, LGTM!

(BTW you can run node ,.bin/prerelease.js to see exactly what happens during the release process; I use this to debug to make sure that the dependencies/version are correctly set right before publish.)

@garrensmith
Copy link
Member Author

Thanks I've cleaned it up. The dependencies / versions keeps appearing because I run the bin/prerelease to make sure I have everything setup correctly. If the tests pass I'm going to merge.

Copy link
Member

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

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

👍 awesome

@nolanlawson nolanlawson merged commit b824fbd into master Feb 22, 2017
@marten-de-vries
Copy link
Member

Very nice to see a monorepo taking shape for pouchdb-server as well!

nolanlawson added a commit that referenced this pull request Feb 24, 2017
@nolanlawson
Copy link
Member

I fixed the mistake I made when I originally merged this, which was that I squash-rebased everything and thus @marten-de-vries didn't get any credit for pouchdb-replicator. That's fixed now; I reverted b824fbd and merged in the pouchdb-replicator branch the traditional way, so the full history is preserved. Thank you @garrensmith for alerting me to my mistake!

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.

5 participants