Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

Add status component #416

Merged
merged 2 commits into from
Apr 29, 2019
Merged

Add status component #416

merged 2 commits into from
Apr 29, 2019

Conversation

rawagner
Copy link
Contributor

@rawagner rawagner commented Apr 26, 2019

Status component is reused for Vm status, Node status and BareMetalHost status

@coveralls
Copy link

coveralls commented Apr 26, 2019

Pull Request Test Coverage Report for Build 1691

  • 53 of 66 (80.3%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 86.091%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/BareMetalHosts/StatusComponents.js 6 7 85.71%
src/components/Status/Status.js 11 12 91.67%
src/utils/status/node/nodeStatus.js 9 10 90.0%
src/components/NodeStatus/NodeStatus.js 12 22 54.55%
Totals Coverage Status
Change from base Build 1446: -0.1%
Covered Lines: 3739
Relevant Lines: 4162

💛 - Coveralls

@rawagner rawagner changed the title [WIP] Add status component Add status component Apr 26, 2019
@rawagner rawagner requested a review from mareklibra April 26, 2019 10:22
<div>This host is under maintenance.</div>
{maintenanceReason && (
<React.Fragment>
<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to use CSS for this?

<div>{maintenanceReason}</div>
</React.Fragment>
)}
<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

likewise

const overlay = (
<Popover id={`${getName(node)}-status-popover`} title="Stopping maintenance">
<div>This host is leaving maintenance. It will rejoin the cluster and resume accepting workloads.</div>
<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

css?

/>
);
default:
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be null returned here?

@@ -21,62 +20,39 @@ import {
getVmStatus,
} from '../../utils/status/vm';
import { getId, getVmImporterPods } from '../../selectors';
import { Status, LinkStatus } from '../Status/Status';
Copy link
Contributor

Choose a reason for hiding this comment

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

just ../Status is enough, let's make use the flexibility given by of the webpack's resolver.

@rawagner
Copy link
Contributor Author

@mareklibra fixed all, see new commit.

@mareklibra mareklibra merged commit 43d9fce into kubevirt:master Apr 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants