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

Add microservices status page link to main page #2356

Merged
merged 1 commit into from
Oct 16, 2021

Conversation

dbelokon
Copy link
Contributor

Issue This PR Addresses

Fixes #2347

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

This PR adds an informational icon on the left to the version number so we can easily access the status of the micro-services by clicking on that icon. It also updates the README file with a status icon linking to the same page.

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR does not include tests because it was a UI styling change.
  • Screenshots:
    Main page
  • Documentation: This PR changes the README file for the user to know the status of the micro-services.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

Very cool! A few things I notice.

README.md Outdated
@@ -2,6 +2,7 @@

[![js-airbnb/prettier-style](https://img.shields.io/badge/code%20style-airbnb%2Fprettier-blue)](https://github.com/airbnb/javascript)
![Node.js CI](https://github.com/Seneca-CDOT/telescope/workflows/node-js-ci/badge.svg)
[![Status](https://img.shields.io/badge/Status-informational)](https://api.telescope.cdot.systems/v1/status)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change this text to API Status

Copy link
Member

Choose a reason for hiding this comment

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

Should we add dev's status as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, good idea. @dbelokon let's add two of these badges:

  1. Prod API Status - https://api.telescope.cdot.systems/v1/status
  2. Dev API Status - https://dev.api.telescope.cdot.systems/v1/status

src/web/src/components/Banner.tsx Outdated Show resolved Hide resolved
@dbelokon
Copy link
Contributor Author

Hello!! I have completed the requested changes. Let me know if there is anything else needed😁

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

Fantastic. Almost done.

README.md Outdated
@@ -2,6 +2,8 @@

[![js-airbnb/prettier-style](https://img.shields.io/badge/code%20style-airbnb%2Fprettier-blue)](https://github.com/airbnb/javascript)
![Node.js CI](https://github.com/Seneca-CDOT/telescope/workflows/node-js-ci/badge.svg)
[![Prod. API Status](https://img.shields.io/badge/Prod._API_Status-informational)](https://api.telescope.cdot.systems/v1/status)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's drop the periods: Prod and Dev are good.

@@ -191,6 +198,9 @@ export default function Banner({ onVisibilityChange }: BannerProps) {
</Typography>
</div>
<div className={classes.icon}>
<a className={classes.infoIcon} href="https://api.telescope.cdot.systems/v1/status">
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a title="API Status" attribute here for an info popup.

@dbelokon
Copy link
Contributor Author

dbelokon commented Oct 14, 2021

Done and done 😎
Let me know if anything else is needed.

Copy link
Member

@manekenpix manekenpix left a comment

Choose a reason for hiding this comment

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

Looking nice!

image

Nit: would it look nicer if the links were in a new line?

@humphd
Copy link
Contributor

humphd commented Oct 14, 2021

Nit: would it look nicer if the links were in a new line?

@manekenpix you want these two badges on a new line?

humphd
humphd previously approved these changes Oct 14, 2021
@manekenpix
Copy link
Member

@humphd yes, I think it'd look better

@dbelokon
Copy link
Contributor Author

Thank you!
I put them on their own line now😁

@dbelokon
Copy link
Contributor Author

image

humphd
humphd previously approved these changes Oct 15, 2021
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

Looks great to me.

@manekenpix
Copy link
Member

Looking great, it just needs a rebase and it's good to go 👍

@dbelokon
Copy link
Contributor Author

Okay, I almost broke my local branch! 😅
I had to reset my topic branch to the commit before everything went wrong, and tried to rebase it one more time.
Let me know if it is fine now!

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

@dbelokon despite whatever issues you had getting this rebased, you did it! Well done 🥳

@humphd humphd merged commit 3e1603b into Seneca-CDOT:master Oct 16, 2021
@dbelokon
Copy link
Contributor Author

Hurrayyy🥳🥳 thank you for everything @humphd 😎

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.

Add link to status service on README.md and in front-end
3 participants