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.

Sign up for GitHub

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

Jump to bottom

add [Sonar] badges for various test metrics #3571

Merged
merged 10 commits into from
Jul 1, 2019
Merged

add [Sonar] badges for various test metrics #3571

merged 10 commits into from
Jul 1, 2019

Conversation

calebcartwright
Copy link
Member

Refs #3236 and #3189

  • Adds first class badge support for the various test result/count/etc. metrics that Sonar supports
  • Refactors transform function in the base class to make it easier in the child service classes that work with multiple metrics

@calebcartwright calebcartwright added the service-badge Accepted and actionable changes, features, and bugs label Jun 15, 2019
@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 June 15, 2019 23:30 Inactive
@shields-ci
Copy link

shields-ci commented Jun 15, 2019 •
edited
Loading

Messages
📖 ✨ Thanks for your contribution to Shields, @calebcartwright!

Generated by 🚫 dangerJS against dd74c91

@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 June 18, 2019 22:37 Inactive
@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 June 21, 2019 02:28 Inactive
@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 June 21, 2019 16:10 Inactive
@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 June 24, 2019 19:05 Inactive
@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 June 28, 2019 21:34 Inactive
chris48s
chris48s reviewed Jun 30, 2019
View reviewed changes
Copy link
Member

@chris48s chris48s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this one has sat around festering for a couple of weeks. Only one comment from me..

services/sonar/sonar-tests.service.js Outdated
})
}

transformTestResults({ json, sonarVersion }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to make transform() and transformTestResults() as 2 seperate methods here?

Seems like it would be more natural to just override transform() and call the function in the parent class with super.transform()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 June 30, 2019 20:55 Inactive
@paulmelnikow paulmelnikow temporarily deployed to shields-staging-pr-3571 July 1, 2019 17:27 Inactive
@calebcartwright calebcartwright merged commit ea86543 into master Jul 1, 2019
@shields-deployment
Copy link

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

@calebcartwright calebcartwright deleted the sonar-tests-badge branch July 1, 2019 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chris48s chris48s chris48s approved these changes

Assignees
No one assigned
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@calebcartwright @shields-ci @chris48s @paulmelnikow