coverallsapp / coverage-reporter

Self-contained, universal coverage uploader binary. Under development.
https://coveralls.io
MIT License
45 stars 14 forks source link

fix Codefresh PR number variable #114

Closed snackattas closed 2 months ago

snackattas commented 3 months ago

:zap: Summary

This PR switches from the Pull Request ID, which is an internal Github ID, to the Pull Request Number, for Codefresh, which will make these links work:

2024-03-18 09 28 17

https://codefresh.io/docs/docs/pipelines/variables/

Here's a similar issue with gitlab that I found, where they also switched over to the github PR number from PR id

:ballot_box_with_check: Checklist

mike-burns commented 3 months ago

We pulled that in from node-coveralls, which still has this discrepency.


In the Coveralls app code, it is used from that link you showed, and it's also used for API calls to GitHub, GitLab, BitBucket, and Stash. I can confirm that those APIs -- the ones we use anyway -- expect a "PR number"-like value, and not an ID.

Looks good to me.

coveralls-official[bot] commented 2 months ago

Pull Request Test Coverage Report for Build 8608271581

Details


Files with Coverage Reduction New Missed Lines %
src/coverage_reporter/api/jobs.cr 1 97.92%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 8608261724: -0.1%
Covered Lines: 900
Relevant Lines: 959

💛 - Coveralls
snackattas commented 2 months ago

@mike-burns

In the Coveralls app code, it is used from that link you showed, and it's also used for API calls to GitHub, GitLab, BitBucket, and Stash. I can confirm that those APIs -- the ones we use anyway -- expect a "PR number"-like value, and not an ID.

They are both numbers though...both integers, so I don't know how one could distinguish one from the other

It's just, one's an internal Github ID, and one corresponds to the Github PR number, so you can build URL links based on it

mike-burns commented 2 months ago

Sorry, that paragraph was an explanation clarifying that I checked the Coveralls backend and confirmed that this PR is correct.

afinetooth commented 2 months ago

@snackattas just doing some housekeeping to confirm this had been released and is working for you.

Thanks, @mike-burns!