Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

Update readme and remove unnecessary items from package.json #17

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Oct 30, 2018

  • Add npm run bootstrap to call lerna bootstrap
  • Update instructions on README
  • Remove unnecessary items from package.json in translation

@williaster

@kristw kristw requested a review from williaster October 30, 2018 20:08
@codecov
Copy link

codecov bot commented Oct 30, 2018

Codecov Report

Merging #17 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #17   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines          89     89           
=====================================
  Hits           89     89

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0108fd...6806636. Read the comment docs.

@kristw kristw added the reviewable Ready for review label Oct 30, 2018
"jed": "^1.1.1"
},
"beemo": {
Copy link
Contributor

Choose a reason for hiding this comment

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

why didn't I remove these?! thanks.


```sh
git clone ...superset-ui && cd superset-ui
npm install
lerna bootstrap
npm run bootstrap
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is okay but yarn is a root dep, and most of the scripts are run with yarn run ....

This might not be as important locally / in the docs, but this seems to be important for the travis build right now. learna bootstrap fails when run in the travis env which I found last week. it seems like travis uses it's own flavor of npm which gets called if you call lerna bootstrap, and it fails the build without a lock file, which we don't have or want. I enabled yarn workspaces for the build-config, and they actually also can do between-package sym linking like lerna bootstrap, so it does what we want.

With the new SIP for moving away from npm we might have to re-think this, but wanted to give more context as to why yarn is around. I'll leave it to you if you want to use npm or yarn here, like I said for docs it probably doesn't matter.

Copy link
Contributor Author

@kristw kristw Oct 31, 2018

Choose a reason for hiding this comment

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

Got it. Thanks for the context. My intention was that I was gonna npm install -g lerna and then saw it as devDependencies here so I added the bootstrap script so I can use the local lerna, plus I just create the generator-superset and want to call lerna bootstrap at the end of scaffolding. It is possible that user may not have global lerna so calling the local version is more sure.

I usually use npm run xxx and yarn xxx interchangeably. The yarn workspace thing sounds interesting though. It would be great to either use that or lerna, just not both.

@kristw kristw merged commit 0fc2fbd into master Oct 31, 2018
@kristw kristw deleted the kristw--readme branch October 31, 2018 05:22
@kristw kristw added reviewable Ready for review and removed reviewable Ready for review labels Nov 13, 2018
kristw added a commit that referenced this pull request Apr 17, 2020
* fix: 🐛 y axis bounds

* fix: clip edge only when necessary

* docs: update storybook

* fix: remove src ref from storybook

* fix: import order
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants