-
Notifications
You must be signed in to change notification settings - Fork 248
1.0 #936
Conversation
lerna-debug.log
Outdated
@@ -0,0 +1,52 @@ | |||
lerna(verbose) GitUtilities.isInitialized () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove this file? Useless diff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously it's too early to make an actual code review, lot's of stuff are not clean :)
Thanks for reporting.
What do you think about using npm namespace?
|
We could keep "phenomic" for a version with good defaults and no config. |
Yes. Using |
Seems it's 14$ per month, can't find a free link for oss right now... I won't pay 170$/y for this for an open source project. |
|
The pain with no scope is when a contributor is added, you need to add npm rights for all packages... And when you create a new package, you need to think to add rights to all members etc. That's the main diff imo. Also it looks "official" (and that's another great thing). |
@MoOx let's just write a script that reads a |
Yeah, I think that's what babel does. |
packages/phenomic-docs/App.js
Outdated
<Router history={ browserHistory }> | ||
<Route component={ (props) => <div><Header />{props.children}</div> }> | ||
<Route path="/" component={ Home } /> | ||
<Route component={ props => <div>{props.children}</div> }> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bloodyowl can you confirm this wrapper is useless here and for /changelog routes below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep
const lists = await Promise.all([ | ||
...post.value.tags.map(tag => req.db.getList(req.params.collection, { limit: limit + 1, lt, reverse }, "tags", tag)), | ||
db.getList(req.params.collection, { limit: limit + 1 }), | ||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bloodyowl can you tell me what lt and reverse are supposed to be/come from?
Also, below I guess it's req.db.getList, not just db.getList? (I am enabling the linter so I am spotting this kind of issue - which allow me to be familiar with the code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, should be db
.
lt
-> less thanreverse
-> reverses the query order
they both come from https://github.com/Level/levelup
I would not go for scoped packages, from recent experience it seems that ultimately not every tooling is ready to read package.json fine with scoped packages (especially yarn) |
6120dec
to
921775a
Compare
I have no opinion on whether it's a good idea or not, but it seems that NPM is now offering free namespaces/organizations/scopes to open source. At least cost should no longer be a consideration. https://medium.com/npm-inc/announcing-free-orgs-48b5f9b39510
|
I already subscribed for one ;) |
|
||
function renderToString(store, { renderProps, redirectLocation }, renderHTML) { | ||
const body = ReactDOMServer.renderToString( | ||
<Provider fetch={ fetch } store={ store }> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bloodyowl can you explain me here how this fetch ("spec" fetch) can do his job here when Provider except another kind of fetch (the one that return .json()). I am probably too tired to understand this (I am finishing to add flow types everywhere in the codebase and this is the only issue left for me :) )
…nfig is no one is found
Flow cannot validate children… :’(
…ew "prop-types" package
linkProperties: { | ||
className: "phenomic-HeadingAnchor", | ||
}, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
poke @ben-eb :)
> | ||
<a | ||
href="#test" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and poke again @ben-eb
…on react tree, or as html
…on react tree, or as html (round 2)
Hey everyone, you can expect a first alpha at the end of the month! I am sure @revolunet will be happy :D |
haha no pressure guys :) but YES i'm very happy thats a great news thank you 👍 |
Bye v0.x, we are going to focus on v1 in master! (tests are broken but no big deal, we will fix this asap) |
This branch is contains current 0.x AND 1.0 (in
packages/*
)If you want to play with alpha version (almost stable architecture architecture - but undocumented yet) you can open NEXT.md and read instructions, it's pretty easy thanks to lerna.
We will iterate here by doing PR on this PR until we are happy. We might eventually push this PR into master sooner and cut a "stable" branch to keep 0.x (for bugfixes only) until this 1.x is out (at least as a beta version).
closes #40
closes #340
closes #358
closes #598
closes #609
closes #630
closes #657
closes #674
closes #713
closes #780
closes #791
closes #806
closes #889
closes #700
This will be closes because not relevant anymore
closes #676
closes #778
Also open the door to #517 #732 #734 #888