Uh oh!
There was an error while loading. Please reload this page.
Remove events - #89
Conversation
| "build:all": "npm run build:js && npm run build:js-dev && npm run build:py", | ||
| "build:watch": "watch 'npm run build:all' src" | ||
| "build:watch": "watch 'npm run build:all' src", | ||
| "test": "python -m unittest tests.test_dash_html_components && python -m unittest tests.test_integration && python -m unittest tests.test_dash_import" |
There was a problem hiding this comment.
Let's update the circleci config to also use this.
dash-html-components/.circleci/config.yml
Line 57 in a2e145e
| .readFileSync(listPath, 'utf8') | ||
| .split('\n') | ||
| .filter(item => !!item); | ||
| .filter(item => Boolean(item)); |
There was a problem hiding this comment.
While at it, please add the npm run lint command to the CI Build step
Marc-Andre-Rivet
left a comment
There was a problem hiding this comment.
Two small tweaks, otherwise lgtm
Marc-Andre-Rivet
commented
Jan 10, 2019
Please create the master -> release-v1 merge after this. |
| python -m unittest tests.test_integration | ||
| python -m unittest tests.test_dash_import | ||
| npm run test | ||
| npm run lint |
There was a problem hiding this comment.
I'd do the opposite and have linting run as early as possible (before npm run clean), to prevent executing a long-ish build and tests for nothing.
There was a problem hiding this comment.
Or in this case, I guess the .js files are also generated -- so after the generation.
There was a problem hiding this comment.
to prevent executing a long-ish build and tests for nothing.
I could move it, but I wouldn't want linter errors to prevent running the rest of the tests. The tests aren't that expensive, seems to me it's worth knowing about all failures together. Personally linter errors are about the last thing I deal with - in the unusual case that they're not caught automatically by my editor, forcing me to fix them right away.
There was a problem hiding this comment.
Fair enough for the ordering. I had scenarios like the table where running the tests takes up to 15 minutes :)
There was a problem hiding this comment.
Actually, I don't think you'll get all the errors as-is. If tests fail, the job will terminate before linting happens. You would need a job for tests and one for linting to always get both sources of failure.
There was a problem hiding this comment.
Good point. Looks like we need a set +e perhaps? Personally I'd rather get all the errors even in cases where the tests take 15 minutes...
There was a problem hiding this comment.
Nice! That should also work.
There was a problem hiding this comment.
OK, my bash-fu is not the best, but I couldn't find a straight shell solution that worked the way I wanted... everything I tried either stopped on first failure or ignored errors if something later passed. npm-run-all and its run-s command (with the -c option) behaves the way I wanted, running everything no matter what, and reporting an error from any part.
Closes#88
Pretty simple in this repo - components already have
n_clicksmatching theclickevent, the only one provided here, so there's really no concern about removing it. Note that rebuilding was done using an updated version ofdash, in its ownno-eventsbranch (which I haven't published yet). But I did a few other little things while I was in here:npm run test173510e - unless I'm missing something and there already is an easy way to run them all?elements.txt- I sorted and deduped them 2f873da