chore(scripts): Scripts Clean Up #26
Conversation
|
Locally the build is not working because |
| @@ -10,7 +10,7 @@ cache: yarn | |||
| # Run the the validate script | |||
| # Temporarily also run the build script to make sure it works | |||
| # (will move this to the release step once that's implemented) | |||
| script: yarn run validate && yarn run build | |||
| script: yarn run validate --ci && yarn run build | |||
There was a problem hiding this comment.
now that you've reorganized this totally works! nice!
There was a problem hiding this comment.
that and the fact that yarn passes flags through, made this part really simple.
| @@ -36,12 +34,10 @@ | |||
| "build": "npm-run-all build:targets gen:declarations", | |||
| "build:targets": "gulp build", | |||
| "prebuild:targets": "rm -rf dist && rm -rf lib", | |||
| "tsc": "tsc", | |||
| "postbuild": "bundlesize", | |||
There was a problem hiding this comment.
I think having it here as postbuild actually makes sense. Cuz it's not actually a "build" step, but something you wanna do afterwards 🎉
There was a problem hiding this comment.
I agree, but it was failing for me. I may have to review the docs on it to know how it works.
package.json
Outdated
| @@ -26,8 +26,6 @@ | |||
| "homepage": "https://github.com/eventbrite/eventbrite-sdk-javascript#readme", | |||
| "license": "MIT", | |||
| "scripts": { | |||
| "check:size": "bundlesize", | |||
| "precheck:size": "yarn gulp build:dist:min", | |||
| "check:static": "npm-run-all --parallel lint tsc", | |||
There was a problem hiding this comment.
Is the plan to remove this once you get Typescript working in Jest? As of right now tsc no longer exists...
There was a problem hiding this comment.
Oh yes, that was my intent, I thought I removed it. I think I put it back since it was not in my checklist. 😄
|
Is this finished? For some reason I thought I was waiting on you to make a change... |
|
Yes, we still need to crack the nut on typescript. |
|
I may be able to find a way break away the tsc update and the script clean up so this can land and backlog the tsc update. It would make them cleaner. |
|
I just submitted this PR (azz/jest-runner-tsc#3) that will unblock this issue once accepted and released. |
|
This should be all set. I did some clean up and made sure the tsc was separate. However, I think I remove all tsc (outside of build). |
|
But now we're not running |
|
With these updates there is no Should I add |
|
The |
|
Awesome! So type-checking will happen in I kinda wanna rename |
|
I completely agree with naming it based on the action and not the tech. I will push an update with that script. |
|
Agreed, but I am ok with that. 😄 @rwholey-eb, Any objections? |
|
For merges, I assume we are following our internal pattern of merging our own after approval. |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I have the starting point for the script clean up. As mentioned the
jest-runner-tscdoes not currently support reading thetsconfig.jsonfile which is leading to errors when added.I am going to put this here for initial review and look into how to update the new runner to take that file into account.
fixes #25