-
Notifications
You must be signed in to change notification settings - Fork 680
Blendid 5.0 #540
base: master
Are you sure you want to change the base?
Blendid 5.0 #540
Conversation
{ | ||
"_developer-comment": "this file to stay empty to ensure CI builds pass. editable .stylelintrc found at root/.stylelintrc", | ||
"rules": {} | ||
} |
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.
Cool. We should add this to our internal react template.
cc @solomonhawk
README.md
Outdated
|
||
## Quick start on a fresh project (empty directory) | ||
```bash | ||
yarn init | ||
yarn add blendid | ||
yarn run blendid -- init | ||
yarn run blendid init | ||
yarn run blendid |
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.
I think this could just be yarn blendid init
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.
i think you are correct. not sure why the run
is in there in the first place, so simplifying it may be better for everyone
rules: [ | ||
TASK_CONFIG.javascripts.eslintLoader, | ||
TASK_CONFIG.javascripts.babelLoader | ||
] |
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.
Small request:
Is it impossible right now to add custom loaders? Could you mix in an otherLoaders
field or something like:
rules: [
TASK_CONFIG.javascripts.eslintLoader,
TASK_CONFIG.javascripts.babelLoader
].concat(TASK_CONFIG.javascript.otherLoaders)
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.
that's a really good point
gulpfile.js/tasks/init.js
Outdated
var projectPath = require('../lib/projectPath') | ||
var merge = require('merge-stream') | ||
var merge = require('merge-stream') |
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.
On our React work, anyway, we've stopped aligning requires. It tends to make diffs really noisy and it's frustrating when a really long import name forces everything over to the right.
Would it be possible to keep the old style?
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.
@davist11 voiced this complaint as well and i think it's valid.
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.
Cool stuff! Should there be an upgrade guide?
@nhunzaker are you thinking the upgrade guide would be a part of the README, or its own document? |
@benjtinsley Usually I've included it in a CHANGELOG.md. I've also seen an UPGRADING.md doc in a few projects. |
this is the total work from the blendid5 feature branch.
changes included: