search

coqui-ai / STT

🐸STT - The deep learning toolkit for Speech-to-Text. Training and deploying STT models has never been so easy.
https://coqui.ai
Mozilla Public License 2.0
2.23k stars 270 forks source link

Fixes for warnings and errors in the build. #2345

Closed morganlittle closed 1 year ago

morganlittle commented 1 year ago

A PR to fix the errors in the latest builds and fix warnings that might cause issues

Update Brew to 3.6.21 to fix Mac build issue Update Build AAR+APK to use JDK 11 to build the build issue, related to https://github.com/android-actions/setup-android/issues/378 Update various actions to v3 or v4 to resolve usage of node12, related to https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/ Update set-output calls to echo to GITHUB_OUTPUT, related to https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/. The only set-output in the actions that I haven't changed is in check_artifact_exists\dist\index.js since I don't know how to change it.

Builds in my fork works.

wasertech commented 1 year ago

Thank you for sharing! We still need to make sure all set-output calls are removed. Not just in check_artifact_exists\dist\index.js but everywhere in the pipeline. I think you did most of the heavy lifting so thank you again.

lissyx commented 1 year ago

The call to fix would be https://github.com/coqui-ai/STT/blob/a694187be4817870e53f5e14b24e16b57dfaa581/.github/actions/check_artifact_exists/main.js#L93 but I dont know how the API evolved.

morganlittle commented 1 year ago

I thought I managed to catch most of the set-output calls, I grepped the pipleline folder for it but I didn't test the tag builds so the build in my repo might not have caught anything on that.

Actually reading https://github.com/actions/toolkit/issues/1218 it looks like the setOutput command might have been changed in @actions/core to use the new method seemlessly, which would be why I didn't see any warning messages related to it.

wasertech commented 1 year ago

Yeah ok so it did change but not in an impactful way? Since https://github.com/actions/toolkit/pull/1178 makes sure we can still call core.setOutput() So we can merge this right?