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: setup turbo #235

Merged
merged 15 commits into from
Apr 6, 2024
Merged

chore: setup turbo #235

merged 15 commits into from
Apr 6, 2024

Conversation

sripwoud
Copy link
Member

@sripwoud sripwoud commented Apr 4, 2024

This PR setups turbo as task handler for the following tasks: formatting, linting, building and testing.
It is not as complex as the proofcarryingdata/zupass#1590 referenced in #207.
But it already speeds up considerably task execution in local environments, and speeds it up somewhat in the ci environments (turbo cache isn't setup yet in ci, but turbo executes all tasks in parallel instead of previously sequential && chaining).

Description

This is only a devops/devtool change.

Before After
none of the tasks related to building, formatting, linting or testing are cached. tasks related to building, formatting, linting or testing are cached.
yarn test:libraries
pic-selected-240404-2321-18
yarn turbo test:libraries
pic-selected-240404-2324-15

Test plan

Check caching

  1. Run one of the tasks related to formatting, linting, building and testing.
  2. Run it again: cache should be hit.
  3. Modify some files listed in the corresponding inputs.
  4. Run it again: cache should be missed

Check tasks dependencies

Now all the scripts (especially the tests and linting ones) should execute successfully on a fresh repo without requiring to run any build scripts manually beforehand.
ex:

for dir in artifacts dist typechain-types node_modules;do find . -type d -name "$dir" -exec rm -rf {} +;done && yarn && yarn test --force
for dir in artifacts dist typechain-types node_modules;do find . -type d -name "$dir" -exec rm -rf {} +;done && yarn && yarn style --force

Related Issue(s)

Closes (fully or partially?) #207

Other information

A combination of root level (//#task) and package level (task) tasks are defined in turbo.json.
As the turbo.json defines custom inputs for all tasks, we loose the default turbo behavior of ignoring gitignored files, and we need to explicitly define ignore globs for irrelevant files.
As this monorepo is quite big and involves lot of config/build/log/cache artifacts, I am not sure the inputs globs I defined are 100% correct.

What this PR does not do
Optimize the GH worklfows (would require setting .turbo cache in the GH actions environment).
I suggest doing this in a separate PR later.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have run yarn style without getting any errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@sripwoud sripwoud self-assigned this Apr 4, 2024
@sripwoud sripwoud marked this pull request as ready for review April 4, 2024 22:57
@sripwoud sripwoud requested a review from cedoor April 4, 2024 22:57
@sripwoud sripwoud added the devops 🔧 Operations management and dev tools label Apr 4, 2024
@sripwoud sripwoud marked this pull request as draft April 5, 2024 07:06
@sripwoud sripwoud marked this pull request as ready for review April 5, 2024 09:06
package.json Show resolved Hide resolved
@cedoor cedoor linked an issue Apr 5, 2024 that may be closed by this pull request
@cedoor
Copy link
Member

cedoor commented Apr 5, 2024

@sripwoud wow thank you so much for this!

Looks like the style job keeps executing indefinitely, isn't it?

.github/workflows/pull-requests.yml Show resolved Hide resolved
package.json Show resolved Hide resolved
use `style` task instead of build, lint, format
use `test` instead of test:{circuits,contracts,libraries}
@sripwoud
Copy link
Member Author

sripwoud commented Apr 5, 2024

@sripwoud wow thank you so much for this!

Looks like the style job keeps executing indefinitely, isn't it?

completes successfully after retriggering it

@sripwoud sripwoud requested a review from cedoor April 5, 2024 14:27
Copy link
Member

@cedoor cedoor left a comment

Choose a reason for hiding this comment

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

LGTM :)

@sripwoud
Copy link
Member Author

sripwoud commented Apr 5, 2024

hmm style workflow is taking forever again. Cancelling and re start fixes it. But something is wrong.
Not sure why (concurrent jobs, turbo cache not being shared or turbo locking some files?). Will need to debug

@sripwoud sripwoud marked this pull request as draft April 5, 2024 18:56
@sripwoud sripwoud force-pushed the chore/turbo branch 2 times, most recently from 38b169d to 1a59448 Compare April 5, 2024 20:12
reorder `dependsOn` tasks
exclude `build` task from `style` script
package.json Show resolved Hide resolved
@sripwoud sripwoud marked this pull request as ready for review April 5, 2024 21:39
@sripwoud
Copy link
Member Author

sripwoud commented Apr 5, 2024

hmm style workflow is taking forever again. Cancelling and re start fixes it. But something is wrong. Not sure why (concurrent jobs, turbo cache not being shared or turbo locking some files?). Will need to debug

@cedoor fixed
Problem was wrong ordering of tasks in dependsOn values, not excluding some output files from inputs, trying to build some things at the same time...

I updated the test plan to cover checking the tasks dependencies are well defined.

@cedoor
Copy link
Member

cedoor commented Apr 6, 2024

hmm style workflow is taking forever again. Cancelling and re start fixes it. But something is wrong. Not sure why (concurrent jobs, turbo cache not being shared or turbo locking some files?). Will need to debug

@cedoor fixed Problem was wrong ordering of tasks in dependsOn values, not excluding some output files from inputs, trying to build some things at the same time...

I updated the test plan to cover checking the tasks dependencies are well defined.

Great! It's ready then 👍🏽

@cedoor cedoor merged commit 950dc5b into main Apr 6, 2024
2 checks passed
Copy link

gitpoap-bot bot commented Apr 6, 2024

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2024 ZK-KIT Contributor:

GitPOAP: 2024 ZK-KIT Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@cedoor cedoor deleted the chore/turbo branch April 6, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops 🔧 Operations management and dev tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider using Turborepo for handling the monorepo
2 participants