-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Run bootstrap before linting on CI #1698
Conversation
Codecov Report
@@ Coverage Diff @@
## release/3.3 #1698 +/- ##
============================================
Coverage 22.35% 22.35%
============================================
Files 325 325
Lines 6523 6523
Branches 813 808 -5
============================================
Hits 1458 1458
- Misses 4459 4473 +14
+ Partials 606 592 -14
Continue to review full report at Codecov.
|
I'm not opposed to this at all, but please state the reasons clearly in the PR, Slack messages get lost over time, and the message you link to does not give a comprehensive picture of why this is exactly needed, thanks 👍 |
I've added some clarifications |
…-lint # Conflicts: # yarn.lock
addons/links/README.md
Outdated
@@ -43,7 +43,7 @@ linkTo('Toggle') // Links to the first story in the 'Toggle' kind | |||
With that, you can link an event in a component to any story in the Storybook. | |||
|
|||
- First parameter is the the story kind name (what you named with `storiesOf`). | |||
- Second (optional) parameter is the story name (what you named with `.add`). If the second parameter is omitted, the link will point to the first story in the given kind. | |||
- Second (optional) parameter is the story name (what you named with `.add`). If the second parameter is omitted, the link will point to the first story in the given kind. |
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.
?
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.
No idea =(
#1810 should make this PR partially obsolete. @ndelangen is there any solid reason why we bootstrap |
In fact, even with #1810 there's still a sense in running bootstrap: it removes a bunch of |
Well the reason for not bootstrapping I would like to be able to bootstrap sections individually. BUt it's a nice to have, I do not oppose including docs into the default bootstrap, but it is unnecessary for 90% of the development that occurs currently. (And even if the code-change is in docs, often it can be worked on from github GUI or without running the docs app to begin with) |
Let‘s measure how much does it add up to clean bootstrap time |
Unfortunately, when I try to add
|
Superseded by #1934 |
See https://storybooks.slack.com/archives/C4Q0S3MQR/p1503243475000043
I've added only those bootsrappers, lack of which produced issues for me. Maybe it's worth to add some more (or remove
bootstrap:docs
)The issues I faced are:
import/extensions
to "error" level is that packageshighlight.js
andfuse.js
are treated by this rule as local files unless they're actually installedeslint-plugin-flowtype
,eslint-plugin-flowtype-errors
, andflow-bin
to root package instead oflib/components
so that they're installed with simpleyarn install
without bootstrappingThe main problem is that all those issues are discoverable only when it actually fails on CI (unless you run
yarn bootstrapp -- --reset
before each commit, after you made the changes)