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

Add selector to check if host can add machine #333

Merged
merged 1 commit into from
Apr 12, 2019

Conversation

jtomasek
Copy link

@jtomasek jtomasek commented Apr 9, 2019

No description provided.

};

export const BaremetalHostStatus = ({ host }) => {
const hostStatus = getHostStatus(host);
Copy link
Contributor

Choose a reason for hiding this comment

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

we found out that for better reusability it is better to decouple status functionality from the UI (e.g. vmStatus is used at many places). Please see utils/status. Could you use this approach here?

Basically it is a function which returns

{
   status, // constant
   ...additionalUsefulData // e.g. message
}

import { getHostStatus } from '../../selectors/host/selectors';

// Status titles. Could be i18n'ed at some point.
const statusTextMap = {
Copy link
Contributor

Choose a reason for hiding this comment

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

+ the string mapping could be done there

};

// Generic status component as a fallback
export const GenericStatus = ({ host }) => <React.Fragment>{getStatusText(getHostStatus(host))}</React.Fragment>;
Copy link
Contributor

Choose a reason for hiding this comment

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

why the React.Fragment ?

@atiratree
Copy link
Contributor

sorry, I see it depends on the other PR now..

@coveralls
Copy link

coveralls commented Apr 11, 2019

Pull Request Test Coverage Report for Build 1187

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 87.345%

Totals Coverage Status
Change from base Build 1185: 0.005%
Covered Lines: 3316
Relevant Lines: 3638

💛 - Coveralls

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1164

  • 48 of 49 (97.96%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 87.416%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/BareMetalHosts/StatusComponents.js 14 15 93.33%
Totals Coverage Status
Change from base Build 1163: 0.07%
Covered Lines: 3307
Relevant Lines: 3626

💛 - Coveralls

Add host machine name selector
@jtomasek jtomasek requested a review from atiratree April 12, 2019 08:32
@rawagner rawagner merged commit c8339af into kubevirt:master Apr 12, 2019
@jtomasek jtomasek deleted the can_add_machine branch April 12, 2019 09:26
@mareklibra mareklibra mentioned this pull request Apr 15, 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.

5 participants