-
Notifications
You must be signed in to change notification settings - Fork 165
Feature: Introduce Storybook + monorepo structure #122
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
Feature: Introduce Storybook + monorepo structure #122
Conversation
Merge `bbc/react-transcript-editor@master` in
Thanks @chrishutchinson , will try to have a look with @jamesdools by the end of the week. Meanwhile travis CI it gives this error It seems like we might need to rethink the Perhaps instrad of |
Thanks @pietrop - looks like huge number of lines of code have changed, although I think most of that is the sample JS/JSON files 😅 Re. the Travis config, it'll definitely need some work. It'd be good to understand what your CI-specific test suite was doing, as it may be possible to just run the regular suite (i.e. Let me know, I can push a change to |
Yeah, push a If I got this right, this should run the test scripts in the packages modules. Worth seeing if it works as is, or if it needs tweaking |
I think we also need to install yarn on travis and we might not need npm? Try this language: node_js
node_js:
- "10"
before_install:
- curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 1.15.2
- export PATH=$HOME/.yarn/bin:$PATH
cache:
yarn: true
install:
- node --version
- yarn install
- yarn workspaces info
script:
- yarn workspaces run test --ci also note
|
if the last one doesn't work, another |
Weird output in Travis:
Absolutely no idea why |
this thread might help? |
Having a look at this today, I feel like this could also do with having an ADR, and updated README, to flesh out a bit more the intentions and workings at high level etc.. |
I'd be in favour of a less flat folder structure for the packages.
It could be:
Thinking of names for the individual modules that would make sense if it's published as standalone. The |
Could we also add the |
@chrishutchinson thanks for this! has been very helpful to look at and use as a guide. |
Is your Pull Request request related to another issue in this repository ?
Initially raised in issue #109.
Describe what the PR does
This PR overhauls the structure of the application, otherwise keeping everything else intact. The reasons for this change are to support more modular use of the components for users who wish to bring their own features and UI, while taking advantage of the great work inside this codebase.
A fairly comprehensive list of changes is below:
npm
may not work..)prettier
on all JavaScript files, and simplifies theeslint
config so there are not conflicts withprettier
s defaults@bbc-transcript-editor/*
namespace, which would need to become a new organisation in NPMState whether the PR is ready for review or whether it needs extra work
This PR is absolutely ready for review, but also very possibly will need further work. That entirely depends 😃
Additional context
I believe there will be some additional work in this PR, specifically:
@bbc-transcript-editor/*
) for the packagesREADME
adapters
andexporters
(still not sure on this)travis
config to support automated packaging and deployment to NPM on merge tomaster
NPM
, as an ES6 module, or as a transpiled ES5 componentOne this PR has been merged, there may be some additional work, notably: