Conversation
package.json
Outdated
| "version": "0.9.0", | ||
| "license": "MIT", | ||
| "author": "Anton Bulyenov <antonb@uber.com>", | ||
| "author": "Anton Bulyenov <visualization@uber.com>", |
There was a problem hiding this comment.
Should probably updated to someone who is not anton, as he isn't a maintainer anymore
There was a problem hiding this comment.
Yes, or more globally, "Visualization team"?
There was a problem hiding this comment.
yea i think that's reasonable
| "build-js": "babel src -d dist --copy-files", | ||
| "build-js-example": "npm run build-js -- --source-maps inline", | ||
| "build-docs": "(node-sass src/example/main.scss example.css) & (browserify -t brfs ./dist/example/main.js > example.js)", | ||
| "build-css": "node-sass src/main.scss main.css", |
There was a problem hiding this comment.
it's my understanding that this is actually necessary as the user needs to require node-modules/react-vis/main.css to get the app running
There was a problem hiding this comment.
I only changed the path of the compiled main css file for it to be inside the dist folder instead
There was a problem hiding this comment.
so will users have require dist/main.css instead?
There was a problem hiding this comment.
Yes, I updated the readme to make it clear
Note that the change of the css path is breaking, but worth in my opinion.
|
Changes Unknown when pulling 6da4d9a on dist into ** on master**. |
| "lint": "eslint src tests examples --ignore-pattern node_modules", | ||
| "test": "tape -r babel-register tests/**/*.js", | ||
| "cover": "nyc --reporter=text --reporter=html --reporter=lcov npm test" | ||
| }, |
There was a problem hiding this comment.
I see that the examples and build-* scripts were removed. Are they no longer needed?
There was a problem hiding this comment.
Yes, they are no longer necessary.
|
Note that the current version of You only need to: And it will build, checkout, commit, push and go back to |
|
Also, please update this PR's description. What's the main goal of this PR? Once it's done, let's leave it at that. This PR is growing extremely large. What's the absolute necessary changes vs nice to haves? |
tests/components.js
Outdated
| data: [[1, 2, 3], [4, 5, 6]] | ||
| }; | ||
|
|
||
| const noop = f => f; |
There was a problem hiding this comment.
prefer screaming snake case for constants
README.md
Outdated
| npm install react-vis --save | ||
|
|
||
| Include the CSS from `./node_modules/react-vis/main` in your HTML page or via SASS: | ||
| Include the builded main CSS file in your HTML page or via SASS: |
balthazar
force-pushed
the
dist
branch
2 times, most recently
from
c5d3e62 to
dc79e1a
Compare
|
Changes Unknown when pulling dc79e1a on dist into ** on master**. |
mcnuttandrew
left a comment
mcnuttandrew
left a comment
There was a problem hiding this comment.
I like all of what this does! It shuffles things around in a reasonable way. The one thing that I would ask, is that in the future you track what you are doing more deeply by filing issues about the changes you want to make.
- move index to dist - beautify readme - global author - changelog in dedicated file - resolve src directory for easier requires - use latest node versions for Travis - arch, es6 refactors - coverage and publish to Coveralls - package files, remove ignore and prepublish - showcase using webpack - add lockfiles - simple doc generation Closes #231. Closes #208.
|
Changes Unknown when pulling 95c1e24 on dist into ** on master**. |
- move index to dist - beautify readme - global author - changelog in dedicated file - resolve src directory for easier requires - use latest node versions for Travis - arch, es6 refactors - coverage and publish to Coveralls - package files, remove ignore and prepublish - showcase using webpack - add lockfiles - simple doc generation Closes #231. Closes #208.
- move index to dist - beautify readme - global author - changelog in dedicated file - resolve src directory for easier requires - use latest node versions for Travis - arch, es6 refactors - coverage and publish to Coveralls - package files, remove ignore and prepublish - showcase using webpack - add lockfiles - simple doc generation Closes uber#231. Closes uber#208.
- move index to dist - beautify readme - global author - changelog in dedicated file - resolve src directory for easier requires - use latest node versions for Travis - arch, es6 refactors - coverage and publish to Coveralls - package files, remove ignore and prepublish - showcase using webpack - add lockfiles - simple doc generation Closes uber#231. Closes uber#208.
5 participants
I abandoned the move of the compiled
main.cssfile, since I can't make it compatible with both sass and vanilla css imports, we'll wait for the next major.This introduces a change of arch by moving stuff around, doing minor es6 refactors, a nice readme, better DX with webpack, linting, named exports, coverage published on Coveralls, dedicated changelog file, and some more 😃
This it will be easier to review separate commits, I'll wait for this to rebase.