8000 Fix scrutinizer & coverage upload by jessedobbelaere · Pull Request #3208 · forkcms/forkcms · GitHub
[go: up one dir, main page]

Skip to content

Fix scrutinizer & coverage upload#3208

Merged
carakas merged 2 commits intoforkcms:masterfrom
jessedobbelaere:fix-code-coverage
Nov 1, 2020
Merged

Fix scrutinizer & coverage upload#3208
carakas merged 2 commits intoforkcms:masterfrom
jessedobbelaere:fix-code-coverage

Conversation

@jessedobbelaere
Copy link
Member
@jessedobbelaere jessedobbelaere commented Oct 31, 2020

Type

  • Enhancement

Resolves the following issues

Pull request description

On the topic of static code analysis

  • Scrutinizer seems to rely on a global config we hardcoded in the settings of the scrutinized admin panel. It seemed a bit outdated... I added the .scrutinizer.yml in this repo, and cleaned it up a bit. Having it in the repo itself might help to keep it more up to date? Are we still sure we benefit from scrutinizer? I actually never looked at it in the past years, but I can't speak for others 🤔 I'd be more in favour of improving our PHPStan integration, which seems to be the more modern approach of static code analysis for PHP? But want to hear opinions to keep or remove it?
  • I removed phpcs from scrutinizer's config since we already have that in our Github Actions.
  • Adding phpstan github annotations: https://twitter.com/OndrejMirtes/status/1278262546819100684

On the topic of code coverage

Scrutinizer is being a pain. It never seems to receive the coverage report we sent, even though it waits a long time for it, and the --verbose logs say that we successfully uploaded the file. Did numerous attempts. I have previous experience with the widely known coverage tool https://codecov.io and that seemed to work out of the box. I got it working in just a few minutes, by using their nice Github workflow action ready for use. I think there's a lot of benefits to codecov. It allows us to set a .codecov.yml with rules, like any PR cannot decrease coverage with more than 1%. And it allows us to browse the code on github with the codecov browser extension and see the source code of fork cms colored with red or green to easily spot code with no tests.

@jessedobbelaere jessedobbelaere force-pushed the fix-code-coverage branch 2 times, most recently from 6aa8a7c to d6aba92 Compare 8000 October 31, 2020 17:46
@codecov
Copy link
codecov bot commented Oct 31, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@8ac593d). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3208   +/-   ##
=========================================
  Coverage          ?   28.21%           
  Complexity        ?     8013           
=========================================
  Files             ?      567           
  Lines             ?    30629           
  Branches          ?        0           
=========================================
  Hits              ?     8642           
  Misses            ?    21987           
  Partials          ?        0           
Flag Coverage Δ Complexity Δ
functional 23.91% <0.00%> (?) 8013.00% <0.00%> (?%)
installer 3.88% <0.00%> (?) 8013.00% <0.00%> (?%)
unit 7.78% <0.00%> (?) 8013.00% <0.00%> (?%)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ac593d...de66e6b. Read the comment docs.

@jessedobbelaere jessedobbelaere marked this pull request as ready for review October 31, 2020 23:17
@jessedobbelaere jessedobbelaere requested a review from a team as a code owner October 31, 2020 23:17
@jessedobbelaere jessedobbelaere changed the title Fix scrutinizer coverage upload Fix scrutinizer & coverage upload Oct 31, 2020
@carakas
Copy link
Member
carakas commented Nov 1, 2020

I actually look at it when I look for things to refactor or when I'm adding new code that it actually gets the A score

@carakas
Copy link
Member
carakas commented Nov 1, 2020

The generated links for fork give a 502 error

@carakas carakas added this to the 5.9.2 milestone Nov 1, 2020
@jessedobbelaere
Copy link
Member Author

The generated links for fork give a 502 error

Yeah not the best first impression for you 😀 It seems to affect other repo's as well, e.g. https://github.com/mirumee/saleor (badge in readme) and h DC02 ttps://codecov.io/github/mirumee/saleor?branch=master . I expect it to be fixed soon

@jessedobbelaere
Copy link
Member Author

Link (https://codecov.io/gh/forkcms/forkcms/pull/3208) is working again @carakas , they had an interruption it seems when I check their status page. However, there's no base report yet (master branch) to compare against, so not that much to be seen on the codecov dashboard yet :-)

@carakas carakas merged commit eb2c782 into forkcms:master Nov 1, 2020
@jessedobbelaere jessedobbelaere deleted the fix-code-coverage branch November 1, 2020 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0