Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CodeClimate] Fix coverage and maintainability score badges #1368

Merged
merged 1 commit into from
Dec 18, 2017

Conversation

PyvesB
Copy link
Member

@PyvesB PyvesB commented Dec 12, 2017

Hello there,

This pull request takes care of issue #1329.

I had a closer look at the API documentation and managed to find a way to retrieve the now missing code coverage score in a different way. As before, the code is making a call to the get repository endpoint, but instead of firing a second call to retrieve the headers of Code Climate's own badges, it's now calling the get snapshot or get test coverage reports endpoint to get the scores. In essence the coverage and maintainability badges are now generated only via API body responses rather than trying to do pattern matching on returned headers, which should hopefully be cleaner and more reliable. 👍

Cheers,

Pyves

@shields-ci
Copy link

shields-ci commented Dec 12, 2017

Messages
📖

✨ Thanks for your contribution to Shields, @PyvesB!

Generated by 🚫 dangerJS

@PyvesB PyvesB changed the title [CodeClimate] Coverage and maintainability score badges fix [CodeClimate] Fix coverage and maintainability score badges Dec 12, 2017
@RedSparr0w
Copy link
Member

Thanks for this, changes look good to me 👍

Think this part of the codeclimate badge may also be broken if you feel like taking a look into it 😄

@RedSparr0w RedSparr0w merged commit 9c2645b into badges:master Dec 18, 2017
@PyvesB
Copy link
Member Author

PyvesB commented Dec 18, 2017

@RedSparr0w: thanks for the review. I thought they were broken at first, but actually after doing some tests locally I think the examples in the homepage are just badly chosen. But this may have changed since I checked two weeks or so ago! ^^

@RedSparr0w
Copy link
Member

I'm not really sure what the other badge is supposed to return,
As if i visit the url that it request
https://codeclimate.com/github/RedSparr0w/node-csgo-parser.png
i just get the maintainability badge:

Also from the looks of it, the wrong header regex is being tested?

@PyvesB
Copy link
Member Author

PyvesB commented Dec 20, 2017

@RedSparr0w: after looking at it again, you're right, other things seem to have broken as well. I'm tempted to get rid of all the broken Code Climate code in server.js, add missing tests and only use the new proper API rather than trying to do pattern matching on headers from their own badges, as this has caused us a lot of trouble recently.

@RedSparr0w
Copy link
Member

@PyvesB Yeah looks like all the codeclimate badges are broken (excl this one)

  • coverage - can probably just include the coverage keyword along with c in this PR's section
  • issues - not sure if there is an endpoint for this?
  • score? - im guessing it is like an overall score based on maintainability + coverage, maybe depreciated

@PyvesB
Copy link
Member Author

PyvesB commented Dec 21, 2017

@RedSparr0w

  • I agree. I do find the naming confusing though, as c is also a coverage, but with a score letter rather than a percentage. Nevertheless we should probably stick with it not to break existing URLs in use.
  • Yes, it can be retrieved via the snapshots endpoint.
  • By the looks of it, score seemed to refer to the old GPA metric. We could use the technical debt ratio instead. There is some information in the documentation here. It can be retrieved via the snapshots as well.

By the looks of it, all five badges will have to make the same first API call and will then all make their second call to either the snapshots or test_reports endpoints. I'll see whether it's possible to put all the Code Climate code in a single server.js function. If it doesn't work out nicely, I'll break things down (possibly test_reports vs. snapshots endpoint), but I would ideally like to avoid duplicating code needed to make the first API call. Once we have separate services, it will be much easier to organise. Any thoughts before I make a start? 😉

@RedSparr0w
Copy link
Member

Nope, that all sounds good to me,

Thanks for all your work on this!
😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants