Formatting in extensions #255
Conversation
Codecov ReportAttention: Patch coverage is
|
|
Performance Ratio: |
|
@maleadt any idea how I can get make my codecov token available in buildkite? I don't think I had codecov turned on when you installed buildkite in this org, so maybe there's something else that we need to do to enable coverage? |
|
You need to add the token as an encrypted secret to the pipeline: https://github.com/JuliaGPU/buildkite?tab=readme-ov-file#using-secrets |
|
Thanks for the pointer @maleadt -- I've updated |
|
Our logic to determine if this is a third-party PR seems to fail: https://github.com/JuliaGPU/buildkite/blob/0dbc7b3422d53d770b8e26d79cbada81a0e6d5ce/image/hooks/environment#L5-L8 Was this repository renamed? |
|
Ah, yes -- the org was renamed from |
|
I updated the repository, and now it does successfully decrypt the token: |
|
Thanks for the assistance with this @maleadt -- strangely it doesn't appear to be the case that the result of running the buildkite CI is showing up with the other CI, and is no longer showing up as one of the CI runs at the bottom of this page. Any idea what might be going on? (Thanks so much for your help with this. I really appreciate it!) |
I assume that with the repository transfer, it's now part of an org that doesn't have the buildkite app yet. So you're back to step one in: https://github.com/JuliaGPU/buildkite?tab=readme-ov-file#steps-for-administrators If you can make me temporarily an admin to the org, I can add the integration again. |
|
Done. |
|
Thanks, you can remove me from the org. The integration was set-up already, so I'm not really sure what's up here. I'll take another look, and if not I'll remove and re-create the pipeline if that's OK. |
|
Turns out the Buildkite integration was still tied to the previous org name, so had to be reinstalled. I've also re-created the pipeline; please push another commit, it should hopefully all work now. |
|
Hmmm it now seems to be the case that the tests are running, codecov is doing something during buildkite's run, but the results don't seem to contribute the the coverage (it's still sitting at zero -- see here. @maleadt does everything look okay on your side of things now? I don't seem to be able to look at the complete CI logs on buildkite, so I'm not sure if there any some warnings I'm missing, or something like that. |
This does not report coverage for |
|
@maleadt seemingly this still isn't working quite correctly -- the coverage for Some things to note:
Is there anything else obvious that you can think of that might be causing the lack of coverage? |
|
Coverage info is being detected and submitted: Not sure what the issue is. Maybe you could try adding some |
|
Thanks for taking a look. The total number of files being uploaded to codecov is 10 -- see here for the latest commit. Although it looks like something has changed in the latest run, because it's now refusing to decrypt the secrets again (apologies for if the package rename messed this up). Either way, I agree with you that it looks like somehow the coverage report is getting lost in transit between the buildkite runner and wherever codecov hosts the results. edit: I'll take a look at the |
Yes, it did. I'll probably have to recreate the pipeline again. |
|
@maleadt when you get a minute, would you mind recreating the pipeline? I'm keen to start tackling this again. |
|
I did that right after my previous comment, and Buildkite reveals that the Codecov secret is successfully decrypted. Or is there anything else missing? |
|
🤦 sorry -- I was looking at the old tapir-dot-jl run, rather than Mooncake. Thanks for sorting that! |
Try new key
|
Hurrah! We have non-zero coverage from the buildkite run. I can now resolve the low coverage number, and then merge this. Thanks again for your assistance @maleadt |
|
Only change since CI last passed is a patch bump, so I'm going to merge. |
For reasons that I'm not entirely clear on, I had really poorly formatted basically all of the package extensions. This PR sorts that out. I'll merge when CI passes. Since there are literally no changes to functionality, I won't bother changing the patch version.
edit: this isn't urgent, so I'm going to try and figure out how to sort coverage out before merging this. I imagine we'll be doing more GPU stuff in the comparatively near future, so now is a good time to get it sorted.