-
Notifications
You must be signed in to change notification settings - Fork 942
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
Inline last-checks into @turf/turf's test script #2690
base: master
Are you sure you want to change the base?
Conversation
Instead of having this be its own step, we can just run it as part of the tests turf-mask had an unused mkdirp dependency that it looked like we wanted to get rid of at some point @turf/turf wasn't reexporting @turf/directional-mean so I added that
@twelch and @mfedderly, can we have a time out before merging this change? Looking at last-checks many of the tests seem to overlap with monorepo linting. Additionally they depend on the built JS files in dist/ which i'm hoping we can avoid generating for local and CI builds. There may be benefits to keeping them in a separate step for the time being. |
@smallsaucepan yeah we can definitely hold this one, I'm curious how you will avoid building on CI and still get typescript type checking done. The weird testing format in @turf/turf has always been somewhat overlapping with the capabilities of monorepolint, it predated when I brought in monorepolint to get things standardized across all of the packages years ago and never fully reconciled it all. |
Thanks for that.
The test:types target in each package runs a |
To be clear I'm not deep enough in the build workings to be able to contribute much to the discussion yet other than learn. I've been trying to only approve smaller build related PRs that don't seem to affect the larger deliberation you've both been having. Just to keep things incrementally improving. If this PR is more than that I apologize. |
That's actually really cool! My only two flags would be making sure that test:types exists everywhere, and that it really does trigger errors on build errors for itself and any packages it depends on if we need that behavior (I worry about something like skipLibCheck causing an issue). |
Done, and done. Check out PR #2702 |
Instead of having this be its own step, we can just run it as part of the tests turf-mask had an unused mkdirp dependency that it looked like we wanted to get rid of at some point @turf/turf wasn't reexporting @turf/directional-mean so I added that