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

Rewrite [uptimerobot] #1891

Merged
merged 19 commits into from
Aug 13, 2018
Merged

Rewrite [uptimerobot] #1891

merged 19 commits into from
Aug 13, 2018

Conversation

paulmelnikow
Copy link
Member

This is based on #1890 and the diff will clean up once that goes in.

@paulmelnikow paulmelnikow added the service-badge New or updated service badge label Aug 10, 2018
@shields-ci
Copy link

shields-ci commented Aug 10, 2018 •
edited
Loading

Messages
📖

✨ Thanks for your contribution to Shields, @paulmelnikow!
📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS

@paulmelnikow paulmelnikow mentioned this pull request Aug 10, 2018
Merged
@chris48s
Copy link
Member

Happy to take the review on this, but as @PyvesB has already reviewed #1890, lets get #1890 in first and rebase this onto it so it is easier to read the diff.

@paulmelnikow
Copy link
Member Author

Sounds good. Want to give that a second 👍 and then merge it?

@paulmelnikow paulmelnikow mentioned this pull request Aug 12, 2018
Closed
chris48s
chris48s reviewed Aug 12, 2018
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.

One minor point, but I think this is looking good otherwise. Its good to be making some progress through the list :)

services/uptimerobot/uptimerobot-base.js Outdated

module.exports = class UptimeRobotBase extends BaseJsonService {
static get category() {
return 'other'
Copy link
Member

Choose a reason for hiding this comment

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

Can we just return 'monitoring' here and remove the category() functions from the child classes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha! Yes. It's funny, I went looking for the category and couldn't find it. I was looking in the wrong place :)

@paulmelnikow paulmelnikow merged commit 0c703c1 into badges:master Aug 13, 2018
@shields-deployment
Copy link

This pull request was merged to master branch. Now this change is waiting for deployment.
Deploys usually happen every few weeks. After deployment changes are copied to gh-pages branch.

This badge displays deployment status:

@paulmelnikow paulmelnikow deleted the rewrite-uptimerobot branch August 13, 2018 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chris48s chris48s chris48s left review comments

Assignees
No one assigned
Labels
service-badge New or updated service badge
Projects
None yet
Milestone
No milestone
Development

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