search

scikit-hep / scikit-hep.github.io

Pages defining the website of the Scikit-HEP project.
https://scikit-hep.org
BSD 3-Clause "New" or "Revised" License
12 stars 15 forks source link

feat: add a developers page on coverage and codecov #245

Closed Saransh-cpp closed 2 years ago

Saransh-cpp commented 2 years ago

This was long due!

The "Coverage for projects written in Python and C++" section is empty right now, and I think it would be better if someone experienced could write that section. I have never done any coverage stuff on projects using Python and C++, but I can take up a more days and learn it on a dummy project.

cc: @henryiii @amangoel185

Saransh-cpp commented 2 years ago

@henryiii, is there a reason for not using a dedicated wrapper action for the Codecov Uploader in CLI11's CI? The docs recommend using the action or an integrity check must be performed once an uploader is manually downloaded.

Using the uploader manually (CLI11)

      - name: Upload coverage
        run: |
          curl -Os https://uploader.codecov.io/latest/linux/codecov
          chmod +x codecov
          ./codecov
        working-directory: build

Using the wrapper action

    - name: Upload coverage
      uses: codecov/codecov-action@v3.1.0
eduardo-rodrigues commented 2 years ago

Morning @Saransh-cpp, let us know when this is ready for review, and do not hesitate to request review explictly from 1-2 people - makes it easier.

Thanks for this update.

henryiii commented 2 years ago

Originally the wrapper action was a bit unstable - it was more likely to fail than using it directly. However, I think it's been fine for a while now, I'd be fine to show the action.

Saransh-cpp commented 2 years ago

The PR is ready for a review (except for the TODO section)!

eduardo-rodrigues commented 2 years ago

[Note that you could have taken all suggestions in a single batch rather than via individual commits.]

Ready from my side. Am about to approve. Let's just wait for a 2nd pair of eyes from Henry.

Thank you for this addition 👍!

henryiii commented 2 years ago

We also should add this to cookie.

eduardo-rodrigues commented 2 years ago

We also should add this to cookie.

Good point. @Saransh-cpp, can you make a follow-up issue for this?

I'm going to merge as I doubt you have rights.

Saransh-cpp commented 2 years ago

Thanks for the review, @eduardo-rodrigues and @henryiii! Working on updating the cookie.

henryiii commented 2 years ago

Done now, thanks @Saransh-cpp!