-
-
Notifications
You must be signed in to change notification settings - Fork 651
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
Rename Tour.options.defaults
to Tour.options.defaultStepOptions
.
#244
Conversation
Corresponds to shipshapecode/shepherd#244.
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.
The defaults
-> defaultStepOptions
parts look good! Just one requested change on the yarn test
stuff.
package.json
Outdated
@@ -34,11 +34,12 @@ | |||
"rewrite-paths": "replace 'SF:.*src' 'SF:src' coverage/lcov.info", | |||
"start": "yarn watch", | |||
"start-test-server": "http-server", | |||
"test": "cross-env NODE_ENV=test webpack --config webpack.test.config.js && yarn mocha-headless-chrome", |
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 don't know if I agree with this change. I typically run yarn test:ci
as the full test suite, to run all unit and acceptance tests, so if you want a yarn test
command, we should just have "test": "yarn test:ci"
, but I'm also not sure we need a yarn test
command, since we can just use yarn test:ci
. I'm okay with it though, if we make it run all the tests.
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 feel like it's useful to have a plain test
command so new developers can instantly see which one is the default -- but I'm totally open to suggestions as to what it should say 😄!
My original attempt was to just run mocha-headless-chrome
, but that wasn't rebuilding the code as I made changes -- so that's how I ended up with the longer set of commands. I just updated again to use yarn test:ci
instead.
3e54f39
to
e230ea9
Compare
e230ea9
to
ac87568
Compare
Corresponds to shipshapecode/shepherd#244.
* Rename `defaults` to `defaultStepOptions`. Corresponds to shipshapecode/shepherd#244. * Update shepherd.js
Closes #240.