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

Fixes for warnings and errors in the build. #2345

Merged
merged 2 commits into from Mar 18, 2023

Conversation

morganlittle
Copy link
Contributor

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 android-actions/setup-android#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.

Fix various issues related to building issues and warnings
- Update Brew to 3.6.21 to fix Mac build issue
- Update Build AAR+APK to use JDK 11
- Update various actions to v3 or v4 to resolve usage of node12
- Update set-output calls to echo to GITHUB_OUTPUT

Morgan Little <mlittle@lakeheadu.ca>
Fix issue in workflow file related to unmerged conflicts

Morgan Little <mlittle@lakeheadu.ca>
@wasertech
Copy link
Collaborator

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
Copy link
Collaborator

lissyx commented Mar 16, 2023

The call to fix would be

core.setOutput("status", artifactStatus);
but I dont know how the API evolved.

@morganlittle
Copy link
Contributor Author

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 actions/toolkit#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
Copy link
Collaborator

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

@wasertech wasertech merged commit 946deb0 into coqui-ai:main Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants