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 a basic host check #268

Closed
wants to merge 1 commit into from
Closed

Add a basic host check #268

wants to merge 1 commit into from

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented Jul 13, 2023

For demo purposes.

@nelsonkopliku nelsonkopliku self-assigned this Jul 13, 2023
@nelsonkopliku nelsonkopliku added enhancement New feature or request check labels Jul 13, 2023
@nelsonkopliku nelsonkopliku changed the title Add a fake host check Add a demo host check Jul 13, 2023
@coveralls
Copy link
Collaborator

coveralls commented Jul 13, 2023

Coverage Status

coverage: 96.477%. remained the same when pulling 4677582 on add-demo-host-check into 1f80bf4 on main.

Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

Hey the check by itself is great!
For development this will be very useful, we may still need to discuss if want to mix real checks with fake checks.
So code wise LGTM, let's hear what the rest thinks about including it or not :D

@nelsonkopliku nelsonkopliku force-pushed the add-demo-host-check branch 3 times, most recently from aae3a26 to 4677582 Compare August 25, 2023 14:04
@nelsonkopliku nelsonkopliku changed the title Add a demo host check Add a basic host check Aug 29, 2023
Copy link

@abravosuse abravosuse left a comment

Choose a reason for hiding this comment

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

From the understanding I have about the check yaml, this looks good to me Nelson...

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

I'd rather not mix fake and real checks.

@nelsonkopliku
Copy link
Member Author

nelsonkopliku commented Aug 29, 2023

I'd rather not mix fake and real checks.

I promise I will delete it as soon as we have a real host check.

Think of it as as a basic noop or something like that 😅

@arbulu89
Copy link
Contributor

I'm with @dottorblaster on this.
Why do we need to push this fake check?
Just to have the demo web updated? I though that the checks in the frontend would be disabled as we do with ascs/ers cluster types, with some sort of Coming next banner. Until we have something "real" to deliver

@nelsonkopliku
Copy link
Member Author

Yes, we'll have the coming soon communication.

Nevertheless before adding the coming soon, we'll deliver a working feature that might be interesting testing.

If we don't like this I can ditch it. It will be just a bit more cumbersome testing host checks without the support of an engineer.

No hard feelings 😄

@stefanotorresi stefanotorresi deleted the add-demo-host-check branch October 25, 2023 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
check enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

6 participants