Load Diplomat and twiggy from cache; update diplomat #974
Conversation
Codecov Report
@@ Coverage Diff @@
## main #974 +/- ##
=======================================
Coverage 75.00% 75.00%
=======================================
Files 216 216
Lines 12737 12737
=======================================
Hits 9554 9554
Misses 3183 3183 Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build e4fe14c9c5dc49f80d12001ada6132a4c348a68e-PR-974
💛 - Coveralls |
| - name: Install Diplomat | ||
| if: steps.diplomat-cache.outputs.cache-hit != 'true' |
There was a problem hiding this comment.
Question: did you test this to make sure that != 'true' is the correct condition? (I don't know if it's correct to quote the true since it is a boolean type in the docs)
There was a problem hiding this comment.
.github/workflows/build-test.yml
Outdated
| path: | | ||
| ~/.cargo/bin/twiggy | ||
| ~/.cargo/bin/twiggy.exe | ||
| key: ${{ runner.os }}-${{ steps.twiggy-version.outputs.hash }} |
There was a problem hiding this comment.
Question (here and above): the example has the key take the form
${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
Your key is only
${{ runner.os }}-${{ steps.twiggy-version.outputs.hash }}
Do you need the middle two tokens ("build" and "env.cache-name") ?
There was a problem hiding this comment.
nah, because the git hash is already uniquely identifying
There was a problem hiding this comment.
I included the tool name in the key name for better debuggability
Fixes #973
Pulls in diplomat update from #968
This has the added benefit of no longer needing to sync Diplomat versions across two files; the single source of truth is
icu_capi's Cargo.toml. You can reinstall the correct version of Diplomat withcargo make diplomat-install, easy peasy 😄Our builds are not bottlenecked on the FFI/WASM builds, but it's always good to have room to grow, and to get failing CI results earlier.