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

Time for a monorepo #1875

Closed
tivac opened this issue Jun 14, 2017 · 7 comments
Closed

Time for a monorepo #1875

tivac opened this issue Jun 14, 2017 · 7 comments
Assignees
Labels
Type: Breaking Change For any feature request or suggestion that could reasonably break existing code
Milestone

Comments

@tivac
Copy link
Contributor

tivac commented Jun 14, 2017

We keep on running into issues with things not being published and the repo root is starting to get messy. I think it's time for a monorepo.

@pygy, @isiahmeadows, @lhorie, anyone feel strongly for or against this plan?

Work has already started here: https://github.com/tivac/mithril.js/tree/monorepo

@tivac tivac added the Type: Breaking Change For any feature request or suggestion that could reasonably break existing code label Jun 14, 2017
@tivac tivac self-assigned this Jun 14, 2017
@pygy
Copy link
Member

pygy commented Jun 14, 2017

I'm for it, but I'd rather wait for #1592 to be merged in if you don't mind (unless git is smart enough to track moving files when rebasing).

@tivac
Copy link
Contributor Author

tivac commented Jun 14, 2017

I'm not in a huge rush, happy to manually merge stuff as it comes in.

@dead-claudia
Copy link
Member

I'm for it, since we're already half-way there now. Just a few nits on that repo:

  1. Why is mithril-querystring separate from mithril? It's not already published independently, and it doesn't have much use case outside Mithril. (Most people looking for a querystring module would just use qs).
  2. You forgot to list mithril-stream as a dependency of mithril.
  3. The tests module seems pretty pointless IMHO. We should probably just move the lone test file to mithril/api/tests, where it would make more sense.

@dead-claudia dead-claudia added this to the 2.0.0 milestone Jun 15, 2017
@tivac
Copy link
Contributor Author

tivac commented Jun 15, 2017

Both mithril and test-utils depend on mithril-querystring. I could introduce a bunch of graph cycles by making test-utils and mithril depend on each other but it makes file explorers and editors freak out. I'll probably just make test-utils depend on a 3rd party parser instead long-term, this was a quick pass to get tests running.

@pygy
Copy link
Member

pygy commented Jun 15, 2017

External dependencies will break the tests/index.html pages.

I thought that mithril did not depend onmithril-stream... does it?

@tivac
Copy link
Contributor Author

tivac commented Jun 15, 2017

It ships stream currently, at least.

https://unpkg.com/[email protected]/stream/stream

I'd be totally fine with ceasing that practice though, I don't think that is a necessary part of mithril core to ship with a npm install.

@dead-claudia
Copy link
Member

@tivac To clarify, I'm referring to this file pointing to a directory that doesn't exist. If we decide to unship it (I'm neutral on it, since I almost never use it personally), then my nit is moot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking Change For any feature request or suggestion that could reasonably break existing code
Projects
Status: Completed/Declined
Development

No branches or pull requests

4 participants