Run check types and build within CI...

hicommonwealth / commonwealth

A platform for decentralized communities

https://commonwealth.im

GNU General Public License v3.0

65 stars 38 forks source link

Run check types and build within CI... #7588

Closed burtonator closed 2 weeks ago

burtonator commented 2 weeks ago

Link to Issue

Closes: https://github.com/hicommonwealth/commonwealth/issues/7587

Description of Changes

just adds the required build commands in CI

"How We Fixed It"

Test Plan

Just verify CI works

Deployment Plan

Other Considerations

kurtisassad commented 2 weeks ago

Can you explain this PR a bit?

The proposed changes have the exact same functionality but no longer check types for discord-bot and snapshot-listener.

Also NO_WEBPACK=true is necessary for the commonwealth build otherwise it will also bundle which is not necessary

timolegros commented 2 weeks ago

The proposed changes have the exact same functionality but no longer check types for discord-bot and snapshot-listener.

They don't have the exact same functionality. yarn workspaces run check-types runs check-types in any existing and future workspace including discord-bot and snapshot-listener.

Also NO_WEBPACK=true is necessary for the commonwealth build otherwise it will also bundle which is not necessary

NO_WEBPACK=true only works if you run build or build-ci from root (execute the build.sh script). yarn workspaces run build runs yarn build in each package specifically so NO_WEBPACK=true does nothing when you run yarn build in packages/commonwealth.

This PR essentially adds type and build checking for all existing and future workspaces without us needed to manually modify CI every time a new package is created. Even more importantly though it adds type checking for libs.

burtonator commented 2 weeks ago

Can you explain this PR a bit?

The proposed changes have the exact same functionality but no longer check types for discord-bot and snapshot-listener.

@kurtisassad I just verified and it runs them on all packages across all workspaces.

It runs for discord-bot and snapshot-listener too and any future packages too (which is nice)

Also NO_WEBPACK=true is necessary for the commonwealth build otherwise it will also bundle which is not necessary

We want it to bundle to detect issues in a production build.

For example, the pnpm branch found a bug where we were missing two polyfills and I'm not sure why we haven't seen this yet.

The closer we can make CI work to production, even if spending a bit more time, I think is worth it to find problems before they go live.

This was influenced by a discussion @timolegros and I were having with my sitemaps code not running check-types plus he noticed a bug in another PR that could have been caught if we ran a real build.

kurtisassad commented 2 weeks ago

If we want to test the prod bundle we need to pass in NODE_ENV=production right?

Otherwise the development webpack is bundled in the e2e pipelines.

timolegros commented 2 weeks ago

If we want to test the prod bundle we need to pass in NODE_ENV=production right?

Otherwise the development webpack is bundled in the e2e pipelines.

Yea this doesn't test actually running the production build. Only building + type checking.

burtonator commented 2 weeks ago

@kurtisassad it's a good point that we should probably test the prod webpack too. dev also... @timolegros and I also discussed running server.js and testing that as we've had breakage with imports and so forth.