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

Finish monorepo merge #197

Closed
9 of 12 tasks
nolanlawson opened this issue Feb 18, 2017 · 45 comments
Closed
9 of 12 tasks

Finish monorepo merge #197

nolanlawson opened this issue Feb 18, 2017 · 45 comments

Comments

@nolanlawson
Copy link
Member

nolanlawson commented Feb 18, 2017

The merge of express-pouchdb and pouchdb-server worked out well, but it'd be even better to merge the other packages in too. Let's track them in this issue.

If any of these seem inappropriate to merge, let me know.

/cc @marten-de-vries @garrensmith


Process for each repository

  • Copy module into packages/node_modules

    git subtree add -P packages/node_modules/$name https://github.com/pouchdb/$name.git master
    
  • move tests into tests/$name

    • Update paths as needed
    • replace tabs with spaces
    • replace import .. from ... with const ... = require('...')
    • replace async/await with Promises
  • in packages/node_modules/$name: delete .gitignore, .travis.yml files

  • in packages/node_modules/$name/package.json remove dependencies, devDependencies, scripts and version (it gets added on release). Update all URLs in the package.json file. If any of the dependencies don’t exist in pouchdb-server’s package.json, move them there.

  • remove $name from the mono repo’s package.json

  • git rebase master -s recursive -X ours to keep commit history

After all repositories got merged

  • review all READMEs
  • make sure all merged repositories are required locally and have been removed from dependencies
@garrensmith
Copy link
Member

garrensmith commented Feb 20, 2017

The way I added each repo was using this:
git subtree add -P packages/node_modules/$name https://github.com/pouchdb/pouchdb-$name.git master

That will bring in the repo with full history

@nolanlawson
Copy link
Member Author

Awesome, thanks for sharing.

@garrensmith
Copy link
Member

Pouchdb-replicator #209

@nolanlawson
Copy link
Member Author

@garrensmith you're now an npm owner on all these packages FYI

@garrensmith
Copy link
Member

Thanks. Once I've fixed the replication url issue's in pouchdb-security I'll do another release.

@gr2m
Copy link
Contributor

gr2m commented Jun 15, 2017

@garrensmith what’s the replication url issue?

@garrensmith
Copy link
Member

@gr2m pouchdb-server couldn't not handle a replication url that was a url with an auth header. That is how Fauxton will create a replication document. But it is now fixed.

@gr2m
Copy link
Contributor

gr2m commented Jun 19, 2017

Can I try to continue your work Herr to finish up the mono repo? Or are you on it right now? Any thing else I need to know besides #197 (comment)? What about tests?

@garrensmith
Copy link
Member

I haven't done any more work on it.
So any continued work will be amazing. In terms of tests, just port over the tests from the original repo. And make sure that the current tests from PouchDB are still passing.

@gr2m
Copy link
Contributor

gr2m commented Jun 26, 2017

can you please let me know if this looks good: #236

I’m moving on with the other repositories

@gr2m
Copy link
Contributor

gr2m commented Jun 28, 2017

I’m currently working on pouchdb-auth which is using async/await for testing. I’ve already started to migrate it work with older node versions using Promises, but maybe we can just set node 7.6 as minimum version for testing? I would prefer the minimum version to be the current node maintenance version, but requiring 7.6 will speed up things. I don’t know if otherp ackages have async/await in their tests, too

@garrensmith
Copy link
Member

garrensmith commented Jun 28, 2017 via email

@gr2m
Copy link
Contributor

gr2m commented Jun 28, 2017

I rewrote pouchdb-auth now, too. I think that was probably the biggest one, so will do the same for the rest.

@gr2m
Copy link
Contributor

gr2m commented Jul 4, 2017

I need help debugging some weird issue in #248 🙏

@gr2m gr2m mentioned this issue Jul 4, 2017
@gr2m
Copy link
Contributor

gr2m commented Jul 4, 2017

PRs done.

I think there are more repositories that @marten-de-vries created for use in PouchDB Server only. I wonder if there is a good way to find them all? e.g. can we go trough all dependencies of pouchdb-server and filter packages where Marten is author?

@gr2m
Copy link
Contributor

gr2m commented Jul 4, 2017

I started one PR for the remaing 3 modules, because the separate PRs had weird issues and I thought it might be some cross referencing foo: #250

@gr2m gr2m mentioned this issue Jul 4, 2017
@loic
Copy link
Contributor

loic commented Oct 16, 2017

Should the following plugin also be moved to the monorepo?
pouchdb-system-db
pouchdb-changeslike-wrapper
pouchdb-bulkdocs-wrapper
pouchdb-req-http-query

All are dependencies of pouchdb-auth / pouchdb-security, and currently the situation is pretty dire when it comes getting a working combination of compatible versions. Having a single branch to run CI on would go a long way.

@loic
Copy link
Contributor

loic commented Jan 21, 2018

Also what about:

They also depend on packages from the monorepo, and are themselves dependencies of the monorepo. Should we move them as well?

@marten-de-vries
Copy link
Member

marten-de-vries commented Jan 21, 2018

pouchdb-seamless-auth builds upon pouchdb-auth. It's not related to express-pouchdb. The only reason I can think of why there might have been a dev dependency is that its test suite also covered/covers pouchdb-auth's logic. I don't particularly care about it anymore myself actually. Not sure if there are any other users out there. The principle behind it is still kind of nice. It makes using pouchdb-auth in the browser much more straightforward. Not sure what to do with it. If it gives no problem, it might be worth keeping it just for the test suite coverage over pouchdb-auth.

couchdb-* are essentially shared code between pouchdb-show, pouchdb-list etc. I would recommend including them in the monorepo for easy access.

@loic
Copy link
Contributor

loic commented Jan 23, 2018

With #288 we are finally seeing the light at the end of the tunnel.

All dependencies of the monorepo that themselves have dependencies from the monorepo are now part of the monorepo (sorry it's a bit of a mouthful). Which means that we now get consistent builds with regards to module versions.

I was planning to merge pouchdb-all-dbs as well, although it's not problematic (it's a dependency of the monorepo that doesn't itself depend on the monorepo), but I noticed that it ships browser builds on the Github repo: https://github.com/pouchdb-community/pouchdb-all-dbs/tree/master/dist and since none of the other packages in the monorepo do that I'm not too sure how to proceed. Since that module is not actually problematic I'm tempted to just punt this effort so we can focus on a release.

Next step is to decide on a release number. The following comments touch on this topic: #197 (comment), #197 (comment)

Here are the version numbers of latest published release for all the modules that now live in the monorepo:

couchdb-eval: 1.0.6
couchdb-objects: 1.0.7
couchdb-render: 1.0.1
couchdb-resp-completer: 1.0.3
express-pouchdb: 2.3.7
http-pouchdb: 2.1.2
pouchdb-auth: 3.0.1
pouchdb-bulkdocs-wrapper: 1.0.2
pouchdb-changeslike-wrapper: 1.0.1
pouchdb-list: 1.1.0
pouchdb-plugin-error: 1.0.1
pouchdb-replicator: 2.3.7
pouchdb-req-http-query: 2.0.0
pouchdb-rewrite: 1.0.7
pouchdb-route: 1.0.3
pouchdb-seamless-auth: 2.0.1
pouchdb-security: 2.3.7
pouchdb-server: 2.3.7
pouchdb-show: 1.0.9
pouchdb-size: 1.2.2
pouchdb-system-db: 1.0.4
pouchdb-update: 1.0.8
pouchdb-validation: 1.2.1
pouchdb-vhost: 1.0.2
pouchdb-wrappers: 1.3.6

My personal opinion is that we should go with the path of least resistance and release as 4.x (basically skipping 3.x of pouchdb-server).

In addition I have a bug fix for pouchdb-auth that I'd like included in the release - what got me in this huge rabbit hole 😛 - (newlogic@60570ab); I just need an hour or so to write a test case, and then I think we can get the release gears going.

marten-de-vries added a commit that referenced this issue Jan 23, 2018
@marten-de-vries
Copy link
Member

Awesome work. I agree, releasing a 4.0 seems to be the path of least resistance.

I don't think there's anything inherently wrong about integrating browser builds for pouchdb-all-dbs in the pouchdb-server build process. In fact, lots of others of the plugins now in the monorepo could profit from that infrastructure (e.g. -show, -list, -update, -auth, -validation, -seamless-auth). But that can wait until someone actually needs it. Let's focus on the release.

@loic
Copy link
Contributor

loic commented Jan 23, 2018

Actually I do need it 😛 I was planning to manually browserify the modules I need.

Out of the packages I listed above, could you identify the definitive list of those that should get a browser build?

If you have some idea of how the build process should work, that would be very helpful. When is the dist directory created, when is it committed, how often is it updated (every commit? immediately before release? immediately after release? etc.).

@marten-de-vries
Copy link
Member

Why not just browserify them? It should work fine. Anyway, if you want browser js file builds, the following (in order of usefulness) were written with the browser in mind:

  • pouchdb-validation
  • pouchdb-show
  • pouchdb-list
  • pouchdb-update
  • pouchdb-rewrite
  • pouchdb-seamless-auth
  • pouchdb-replicator
  • pouchdb-vhost
  • http-pouchdb
  • pouchdb-security
  • pouchdb-auth

@loic
Copy link
Contributor

loic commented Jan 23, 2018

Why not just browserify them?

I mean, if we want to maintain browserified modules committed in the repo, how should we go at it? I've never maintained JavaScript libraries, so I'm not sure when those built files are updated, etc.

@marten-de-vries
Copy link
Member

Ah, I think the ideal process would mirror the pouchdb/pouchdb repo. There would be a script that builds the plugins into JS bundles using browserify/rollup/whatever (probably browserify as the code is not as ES 6-ish as pouchdb's, something like browserify -s show -o pouchdb.show.js should do the job). This dist directory would normally not be committed, but it would be by the release script: https://github.com/pouchdb/pouchdb/blob/master/bin/release.sh#L50

That leaves the development branches clear, while still making it possible to download scripts on release.

@loic
Copy link
Contributor

loic commented Jan 24, 2018

The release.sh of pouchdb does a lot more than the pouchdb-server one

It creates git commits, git tags, etc. I'm not sure how releasing was tackled on pouchdb-server until now, should I go ahead and implement those in release.sh or is that going to disrupt the current process?

@marten-de-vries
Copy link
Member

I think the current process expected git commits/tags etc. to be made manually. Otherwise we would have seen scripts in the repo handling it. So 'upgrading' release.sh should not cause any trouble.

I did start to wonder about me having signed off on updating require()'s from require('pouchdb-show') to require('../pouchdb-show') (or something similar) today, by the way. That might break plugins when they are used as (published) npm modules, right? I think such a path cannot be resolved in that case.

@loic
Copy link
Contributor

loic commented Jan 24, 2018

I did start to wonder about me having signed off on updating require()'s from require('pouchdb-show') to require('../pouchdb-show') (or something similar) today, by the way. That might break plugins when they are used as (published) npm modules, right? I think such a path cannot be resolved in that case.

Do you mean in my commits? Normally in the PR that I made '../' is only used in tests, so that shouldn't cause any issues on published modules. I saw this technique in existing test modules so I kept it in the new modules I merged, but I actually wondered if it should just be regular absolute require, especially now that the dependency tree is clean and that we don't have to worry about getting different versions.

@marten-de-vries
Copy link
Member

Ah, I remembered I read that pattern somewhere while reviewing but if it's test-only it's fine. Never mind then. 😄

@marten-de-vries
Copy link
Member

#291 and #292 should also be included in the release. They're relatively small, but fix an important security issue.

@loic
Copy link
Contributor

loic commented Jan 27, 2018

Looks like we are in good shape for a release, wdyt @marten-de-vries?

I checked and you have npm access to all vendored package, so if I didn't mess up too badly release.sh, all we need is npm run release.

In addition it would be lovely if we can issue Github releases to host the browser builds like https://github.com/pouchdb/pouchdb/releases. I looked a bit what it would take to automate, but if I'm not wrong, it's done manually for Pouchdb?

@marten-de-vries
Copy link
Member

I think the Github release is done manually. But as there is a dist directory, it should not be too much of a problem. The script is currently running.

@marten-de-vries
Copy link
Member

It all went pretty smoothly, including manually adding the JS bundles to the release. One thing that would make it easier in the future might be to have a global dist directory for the whole monorepo, but it's certainly doable this way too.

See also the release page.

Now guess what? I get to close our most-commented issue! Thanks for making that possible, @loic and @gr2m.

@loic
Copy link
Contributor

loic commented Jan 27, 2018

Awesome! Thanks a lot @marten-de-vries for shepherding this effort, and @gr2m for paving the way.

@gr2m
Copy link
Contributor

gr2m commented Jan 27, 2018

🏁🏎️💨w00p w00p this is awesome, great work everyone, thanks for pushing it through

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants