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

Migrate [Coverity] to new service model #2550

Merged
merged 4 commits into from
Dec 18, 2018

Conversation

calebcartwright
Copy link
Member

@calebcartwright calebcartwright commented Dec 18, 2018

Everything I've seen indicates that Coverity On Demand has been decommissioned for a while, although I haven't seen any official announcements from Coverity/Snyopsys. If anyone knows otherwise please let me know, but for now I believe deprecation is the best course of action.

All of the on-demand endpoints are down/inaccessible:

And the few online references I have found for Coverity On Demand speak about it in the past tense, like this from the Coverity Wikipedia page:

Coverity Code Advisor on Demand was a cloud hosted version of Coverity Code Advisor.

@shields-ci
Copy link

shields-ci commented Dec 18, 2018

Warnings
⚠️

This PR modified helper functions in lib/ but not accompanying tests.
That's okay so long as it's refactoring existing code.

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

Generated by 🚫 dangerJS against 06992fd

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.

Thanks for having a look at this one. I've left a few minor comments, but its looking good

message: 'passed',
})
)
.expectJSON({ name: 'coverity', value: 'passing' })
Copy link
Member

Choose a reason for hiding this comment

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

If you want to mock a response for each case, we could usefully make the calls with the ?style=_shields_test param here and test the colouring logic too. There's an example of this in the docs https://github.com/badges/shields/blob/master/doc/service-tests.md#5-mocking-responses

Copy link
Member Author

Choose a reason for hiding this comment

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

Excellent! I was wondering about color validation. Will add this (I should probably update some other service tests to include this as well)

Copy link
Member

Choose a reason for hiding this comment

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

Another approach which can be taken here is a Sazerac-based test of render in a .spec.js file.

Copy link
Member Author

@calebcartwright calebcartwright Dec 18, 2018

Choose a reason for hiding this comment

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

Added color validation via ?style=shields_test, will keep in mind the Sazerac-based tests too going forward

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.

nice one :)

@chris48s chris48s merged commit a1150ef into badges:master Dec 18, 2018
@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:

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.

4 participants