Add a component for creating baremetal hosts #292

honza · 2019-03-26T17:34:43Z

coveralls · 2019-03-26T17:38:38Z

Pull Request Test Coverage Report for Build 1185

6 of 17 (35.29%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.2%) to 87.714%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/components/CreateBaremetalHostDialog/CreateBaremetalHostDialog.js	4	15	26.67%

Totals
Change from base Build 1157:	-0.2%
Covered Lines:	2986
Relevant Lines:	3264

💛 - Coveralls

atiratree · 2019-03-26T19:52:13Z

src/k8s/request.js

@@ -504,3 +512,10 @@ const getDefaultInterface = (vm, template) => {
  const defaultInterfaceName = getTemplateAnnotations(template, ANNOTATION_DEFAULT_NETWORK);
  return getDevice(vm, 'interfaces', defaultInterfaceName);
 };
+
+export const createBaremetalHost = async (k8sCreate, data) => {


you can use object deconstruction here

Please create a new file for this. It is crowded here already. I suggest k8s/requests/host/.. or something similar

atiratree · 2019-03-26T19:53:49Z

src/components/CreateBaremetalHost/CreateBaremetalHost.js

+  },
+};
+
+export class CreateBaremetalHost extends React.Component {


I think adding a Dialog to the name would make it more clear what to expect from this component

atiratree · 2019-03-26T19:57:42Z

src/components/CreateBaremetalHost/CreateBaremetalHost.js

+    const namespace = this.props.selectedNamespace.metadata.name;
+    const secretName = `${data.name.value}-secret`;
+
+    const secret = {


please can you move creation of these objects to createBaremetalHost function?

atiratree · 2019-03-26T19:58:39Z

src/components/CreateBaremetalHost/CreateBaremetalHost.js

+        </Modal.Header>
+        <Modal.Body>
+          <Col sm={12}>
+            <p style={{ marginTop: '20px' }}>


please no inline styles

atiratree · 2019-03-26T20:03:34Z

src/components/CreateBaremetalHost/CreateBaremetalHost.js

+      isSubmitting: true,
+    }));
+    const data = this.state.form.value;
+    const namespace = this.props.selectedNamespace.metadata.name;


you can use getName from selectors

honza · 2019-03-27T00:42:52Z

Great points, thanks

src/components/CreateBaremetalHostDialog/CreateBaremetalHostDialog.js

src/k8s/requests/hosts/index.js

atiratree · 2019-03-27T10:46:50Z

src/k8s/requests/hosts/index.js

+  const namespace = getName(selectedNamespace);
+  const secretName = `${formData.name.value}-secret`;
+
+  const secret = {


would be nice to split creation of each object into a function (e.g. getSecret) for better readability

src/components/CreateBaremetalHostDialog/CreateBaremetalHostDialog.js

src/k8s/requests/hosts/index.js

src/components/CreateBaremetalHostDialog/CreateBaremetalHostDialog.js

knowncitizen · 2019-04-01T14:55:48Z

Nice

kubevirt-bot added the size/L label Mar 26, 2019

atiratree suggested changes Mar 26, 2019

View reviewed changes

honza force-pushed the create-baremetal-host branch from 4c0463d to f760d7e Compare March 27, 2019 00:42

atiratree suggested changes Mar 27, 2019

View reviewed changes

honza force-pushed the create-baremetal-host branch from f760d7e to b76484e Compare March 27, 2019 18:02

atiratree reviewed Apr 1, 2019

View reviewed changes

src/components/CreateBaremetalHostDialog/CreateBaremetalHostDialog.js Outdated Show resolved Hide resolved

atiratree approved these changes Apr 1, 2019

View reviewed changes

rawagner suggested changes Apr 1, 2019

View reviewed changes

src/k8s/requests/hosts/index.js Outdated Show resolved Hide resolved

honza force-pushed the create-baremetal-host branch 2 times, most recently from 01ea7f4 to d3f6e95 Compare April 1, 2019 11:19

rawagner reviewed Apr 1, 2019

View reviewed changes

src/components/CreateBaremetalHostDialog/CreateBaremetalHostDialog.js Show resolved Hide resolved

rawagner approved these changes Apr 1, 2019

View reviewed changes

jtomasek approved these changes Apr 1, 2019

View reviewed changes

honza force-pushed the create-baremetal-host branch from d3f6e95 to 410ce70 Compare April 1, 2019 14:39

Add a component for creating baremetal hosts

6394b39

honza force-pushed the create-baremetal-host branch from 410ce70 to 6394b39 Compare April 1, 2019 14:51

rawagner merged commit 4c7b8c2 into kubevirt:master Apr 1, 2019

honza deleted the create-baremetal-host branch April 1, 2019 14:56

cloudbehl pushed a commit to gnehapk/web-ui-components that referenced this pull request Apr 1, 2019

Add a component for creating baremetal hosts (kubevirt#292)

d3674f6

Add a component for creating baremetal hosts #292

Add a component for creating baremetal hosts #292

Uh oh!

Conversation

honza commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 1185

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

honza commented Mar 27, 2019

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knowncitizen commented Apr 1, 2019

Uh oh!

Uh oh!

honza commented Mar 26, 2019 •

edited

Loading

coveralls commented Mar 26, 2019 •

edited

Loading