celestiaorg / nmt

Namespaced Merkle Tree
Apache License 2.0
107 stars 39 forks source link

ci: add secrets.CODECOV_TOKEN to codecov #255

Closed ramin closed 2 months ago

ramin commented 2 months ago

Closes https://github.com/celestiaorg/nmt/issues/250

Overview

@staheri14 noticed rate limiting in CI when sending data to CODECOV. This uses the org wide secret to ensure we are free from public usage/rate limiting

ramin commented 2 months ago

FYI @MSevey this is still being odd, can you verify the secret is available here and i believe there might be some admin on the codecov side maybe, see: https://github.com/celestiaorg/nmt/actions/runs/9077675099/job/24943089833?pr=255#step:8:33

rootulp commented 2 months ago

there might be some admin on the codecov side maybe

+1 per this line maybe we need to enable the NMT in CodeCov UI so I deactivated and activated the repo.

I can't change the default branch context to main perhaps b/c we haven't had a successful CodeCov upload from main yet

Screenshot 2024-05-14 at 8 59 09 AM

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 66.24%. Comparing base (749d39b) to head (aebead0).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #255 +/- ## ========================================== - Coverage 68.30% 66.24% -2.06% ========================================== Files 6 6 Lines 1325 1022 -303 ========================================== - Hits 905 677 -228 + Misses 403 328 -75 Partials 17 17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ramin commented 2 months ago

@rootulp bumped the action version to @v4 and that took care of some stuff, seems like it fails on a "test coverage regression" which I assume is maybe it comparing head to wrong primary branch? Otherwise, whatever was acting wild stopped

staheri14 commented 2 months ago

Does anything hinder this PR from being merged?

rootulp commented 2 months ago

IMO no, I didn't hit merge b/c we usually let PR authors merge their own PRs. Seems safe to merge it