Skip to content
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

chore(README.md, package.json): Update npm script documentation #3190

Merged
merged 2 commits into from
Jan 12, 2018

Conversation

martinsik
Copy link
Contributor

I've noticed there were some leftovers from #2851.

I added more comments to appropriate lines in code.

- build_perf: builds ES6, CommonJS, then global, then runs the performance tests with `protractor`
- build_docs: generates API documentation from `dist/es6` to `dist/docs`
- build_cover: runs `istanbul` code coverage against test cases
- test: runs tests with `jasmine`, must have built prior to running.
- test: runs tests with `mocha`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not using jasmine any more and the tests don't need to be build first.

@@ -172,7 +171,8 @@ npm run build_all
## Performance Tests

Run `npm run build_perf` or `npm run perf` to run the performance tests with `protractor`.
Run `npm run perf_micro` to run micro performance test benchmarking operator.

Run `npm run perf_micro [operator]` to run micro performance test benchmarking operator.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this was intended to be on a new line (just Markdown requires two new lines). I also added [operator] to make it obvious you can run performance tests on a single operator. I'm not sure the description here https://github.com/ReactiveX/rxjs/blob/master/CONTRIBUTING.md#micro is still accurate.

Also this section should probably mention that you need to run npm run build_all first.

"test_browser": "Execute mocha test runner on browser against existing test spec build",
"test": "Clean up existing test spec build, build test spec and execute mocha test runner",
"test": "Execute mocha test runner",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't cleanup anything and doesn't need to be build prior.

@@ -117,7 +115,7 @@
"test:cover": "cross-env TS_NODE_FAST=true nyc npm test",
"test:circular": "dependency-cruise --validate .dependency-cruiser.json -x \"^node_modules\" src",
"tests2png": "tsc && mkdirp tmp/docs/img && mkdirp spec-js/support && shx cp spec/support/*.opts spec-js/support/ && mocha --opts spec/support/tests2png.opts spec-js",
"watch": "watch \"echo triggering build && npm run build_test && echo build completed\" src -d -u -w=15"
"watch": "watch \"echo triggering build && npm run test && echo build completed\" src -d -u -w=15"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if watch is supposed to always run all tests.

@@ -153,11 +153,10 @@ The build and test structure is fairly primitive at the moment. There are variou
- build_amd: Transpiles the ES6 files from `dist/es6` to `dist/amd`
- build_global: Transpiles/Bundles the CommonJS files from `dist/cjs` to `dist/global/Rx.js`
- build_all: Performs all of the above in the proper order.
- build_test: builds ES6, then CommonJS, then runs the tests with `jasmine`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

build_test doesn't exist any more.

@rxjs-bot
Copy link

Messages
📖

CJS: 1383.5KB, global: 752.4KB (gzipped: 120.7KB), min: 145.3KB (gzipped: 31.4KB)

Generated by 🚫 dangerJS

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.079% when pulling 819f681 on martinsik:update-readme-and-packagejson into d5ced68 on ReactiveX:master.

@benlesh benlesh merged commit 97cf011 into ReactiveX:master Jan 12, 2018
@benlesh
Copy link
Member

benlesh commented Jan 12, 2018

Thank you, @martinsik .... sorry for the slow merge

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
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.

4 participants