Bug 1829542: Add mount guest agent tools to vm wizard #5490

yaacov · 2020-05-19T14:13:09Z

When Selecting a Windows Operating System in the VM creation wizard there is no indication that Guess Tools have been added to the virtual hardware section.

Ref:
http://openshift.github.io/openshift-origin-design/designs/virtualization/4.3/create-vm/step-1-genaral/#operating-system-field

Screenshot:

openshift-ci-robot · 2020-05-19T14:13:16Z

@yaacov: This pull request references Bugzilla bug 1829542, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1829542: Add mount guest agent tools to vm wizard

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yaacov · 2020-05-19T14:14:10Z

@suomiy @glekner @irosenzw @pcbailey please review

yaacov · 2020-05-19T14:37:48Z

/test images

atiratree · 2020-05-19T15:14:41Z

/assign

atiratree · 2020-05-20T14:04:54Z

...-plugin/src/components/create-vm-wizard/redux/initial-state/vm-settings-tab-initial-state.ts

@@ -52,6 +52,10 @@ export const getInitialVmSettings = (data: CommonData): VMSettings => {
    [VMSettingsField.OPERATING_SYSTEM]: {
      isRequired: asRequired(true),
    },
+    [VMSettingsField.MOUNT_WINDOWS_GUEST_TOOLS]: {
+      value: true,
+      isRequired: asRequired(false),


no need, for isRequired false value,

also we should have this initially hidden

atiratree · 2020-05-20T14:06:22Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

@@ -113,6 +122,22 @@ export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
            </FormSelect>
          </FormField>
        </FormFieldRow>
+        {isWindows && (
+          <FormFieldRow


this clashes with isHidden wizard logic. We should always render FormFieldRow and set isHidden flag when required

The main advantage of keeping all these variables (hidden, disabled, required) in redux is that we can easily monitor the wizard (through ReduxDevTools) and see all the changes it makes with each change

atiratree · 2020-05-20T14:07:07Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

@@ -91,6 +93,13 @@ export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
      }
    }

+    const isWindows = os && os.startsWith('win');
+    const mountWindowsGuestToolsMessage = (


can we inline it? We would have 3 lines less of code

atiratree · 2020-05-20T14:47:16Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

+                onChange={(v) => onChange(VMSettingsField.MOUNT_WINDOWS_GUEST_TOOLS, v)}
+                label="Mount Windows guest tools"
+              />
+              {mountWindowsGuestToolsMessage}


this shouldn't be inside the <FormField/>

It's inside the form because it's a hint message for this form field, it is part of this form field

atiratree · 2020-05-20T14:48:20Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

+          >
+            <FormField>
+              <Checkbox
+                id={getFieldId(VMSettingsField.MOUNT_WINDOWS_GUEST_TOOLS, 'checkbox')}


the id should be defined in idResolver - you can add append checkbox there

atiratree · 2020-05-20T14:54:19Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

@@ -141,8 +166,9 @@ type OSFlavorProps = {
  commonTemplates: any;
  flavorField: any;
  operatinSystemField: any;
+  mountWindoesGuestTools: any;


atiratree · 2020-05-20T14:54:35Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

  userTemplate: string;
  workloadProfile: string;
  openshiftFlag: boolean;
-  onChange: (key: string, value: string) => void;
+  onChange: (key: string, value: any) => void;


can we do string | boolean instead?

atiratree · 2020-05-20T14:55:35Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

@@ -1,6 +1,7 @@
 import * as _ from 'lodash';
 import * as React from 'react';
-import { FormSelect, FormSelectOption } from '@patternfly/react-core';
+import { FormSelect, FormSelectOption, Checkbox } from '@patternfly/react-core';
+import { getFieldId } from '@console/shared';


please try to import the whole path when you do import from shared - it is very easy to get into trouble doing this

atiratree · 2020-05-20T14:56:55Z

...es/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/vm-settings-tab.scss

@@ -6,3 +6,7 @@
 .kubevirt-create-vm-modal__memory-row {
  margin-right: 0.5em;
 }
+
+.kubevirt-create-vm-modal__sub-field-row {


where is this class used?

it is adding a little space between the help comment and the checkbox

Are you using the patternfly checkbox component? It shouldn't require any special styles

@matthewcarleton

Are you using the patternfly checkbox component?

yes

It shouldn't require any special styles

The wizard and wizard components add styling and layout.

It is not the wizard.

This is due to the bootstrap _forms.scss styling

wchich adds this

// Position radios and checkboxes better input[type="radio"], input[type="checkbox"] { margin: 4px 0 0; margin-top: 1px \9; // IE8-9 line-height: normal; }

so we either need to add css to counter that or disable global bootstrap

atiratree · 2020-05-20T15:07:52Z

...rt-plugin/src/components/create-vm-wizard/redux/state-update/vm-settings-tab-state-update.ts

@@ -147,19 +147,27 @@ const flavorUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) =>

 const osUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {


I think we could make the logic a bit more reconcilable here.

Let's do two updaters

osUpdater

called first

resolves the MOUNT_WINDOWS_GUEST_TOOLS field according to the os (win, etc)

should set value (false if linux for example) and isHidden

winToolsUpdater

called after osUpdater

should create storage if MOUNT_WINDOWS_GUEST_TOOLS value is true but storage does not exist

should remove storage if MOUNT_WINDOWS_GUEST_TOOLS is false and storage exists

atiratree · 2020-05-21T10:11:39Z

...-plugin/src/components/create-vm-wizard/redux/initial-state/vm-settings-tab-initial-state.ts

@@ -52,6 +52,10 @@ export const getInitialVmSettings = (data: CommonData): VMSettings => {
    [VMSettingsField.OPERATING_SYSTEM]: {
      isRequired: asRequired(true),
    },
+    [VMSettingsField.MOUNT_WINDOWS_GUEST_TOOLS]: {
+      value: false,
+      isHidden: asHidden(true),


Suggested change

isHidden: asHidden(true),

isHidden: asHidden(true, VMSettingsField.OPERATING_SYSTEM),

It is also good to specify a reason why this is hidden (if we know it). So it is easier for multiple updaters to change the isHidden property (and give their reasons)

It is also then easier to debug this and see who has hidden this field

atiratree · 2020-05-21T10:12:57Z

...rt-plugin/src/components/create-vm-wizard/redux/state-update/vm-settings-tab-state-update.ts

@@ -152,14 +152,34 @@ const osUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {
  }
  const os = iGetVmSettingValue(state, id, VMSettingsField.OPERATING_SYSTEM);
  const isWindows = os && os.startsWith('win');


can we convert this to boolean?

atiratree · 2020-05-21T10:13:19Z

...rt-plugin/src/components/create-vm-wizard/redux/state-update/vm-settings-tab-state-update.ts

+    vmWizardInternalActions[InternalActionType.UpdateVmSettingsField](
+      id,
+      VMSettingsField.MOUNT_WINDOWS_GUEST_TOOLS,
+      { isHidden: asHidden(!isWindows), value: isWindows },


let's use the os reason here as mentioned

atiratree · 2020-05-21T11:11:25Z

...rt-plugin/src/components/create-vm-wizard/redux/state-update/vm-settings-tab-state-update.ts

@@ -152,14 +152,34 @@ const osUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {
  }
  const os = iGetVmSettingValue(state, id, VMSettingsField.OPERATING_SYSTEM);
  const isWindows = os && os.startsWith('win');
+
+  dispatch(


IMO, we should also consider the vm import part here.

can we change the value to false when isProviderImport? But keep it visible in case some import providers (possibly vmWare) wish to use it.

In case of ovirt provider it is actually impossible to use this field so can we hide it in ovirt-state-update.ts?

We should also register field reset for MOUNT_WINDOWS_GUEST_TOOLS in each provider - look for vmFieldUpdate

I don't have an env to test and check this scenarios ...

@suomiy can you take it as a follow up PR ?

should be few lines, just use vmFieldUpdate in the same way like for other VM fields and also register isHidden in ovirt provider in the same spot/update.

I can test your PR, once you have it.

added a gate to not mount when in import view

pcbailey · 2020-05-21T11:51:07Z

Would it be possible to make the Virtual Hardware in the message a link to the Virtual Hardware tab? Or is that something we don't want to do for some reason? Thoughts?

@matthewcarleton What do you think?

yaacov · 2020-05-21T13:45:23Z

...rt-plugin/src/components/create-vm-wizard/redux/state-update/vm-settings-tab-state-update.ts

-  const os = iGetVmSettingValue(state, id, VMSettingsField.OPERATING_SYSTEM);
-  const isWindows = os && os.startsWith('win');
+  const os: string = iGetVmSettingValue(state, id, VMSettingsField.OPERATING_SYSTEM);
+  const isWindows: boolean = os && os.startsWith('win');


matthewcarleton · 2020-05-21T19:52:31Z

Would it be possible to make the Virtual Hardware in the message a link to the Virtual Hardware tab? Or is that something we don't want to do for some reason? Thoughts?

@matthewcarleton What do you think?

I think that would be a nice addition!

matthewcarleton · 2020-05-21T19:53:54Z

When Selecting a Windows Operating System in the VM creation wizard there is no indication that Guess Tools have been added to the virtual hardware section.

Ref:
http://openshift.github.io/openshift-origin-design/designs/virtualization/4.3/create-vm/step-1-genaral/#operating-system-field

Screenshot:

the spacing feels a bit off from the design. Can we get it closer?

atiratree · 2020-05-22T15:26:21Z

Can we have a link to that tab as well?
Something similar to what we do in import providers?

matthewcarleton · 2020-05-22T16:24:02Z

Can we have a link to that tab as well?
Something similar to what we do in import providers?

I think this is what @pcbailey is asking for to right?

atiratree · 2020-05-22T17:03:29Z

I think this is what @pcbailey is asking for to right?

ok cool, I was just making sure after seeing the design

@yaacov
btw you can take a look on how this is implemented in wizard-errors.tsx

pcbailey · 2020-05-26T13:14:21Z

@matthewcarleton @suomiy Yup! This is exactly what I was asking to have done. =)

openshift-ci-robot · 2020-05-27T14:33:11Z

@yaacov: This pull request references Bugzilla bug 1829542, which is invalid:

expected the bug to target the "4.5.0" release, but it targets "4.6.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1829542: Add mount guest agent tools to vm wizard

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-08-05T15:09:03Z

@yaacov: This pull request references Bugzilla bug 1829542, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1829542: Add mount guest agent tools to vm wizard

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yaacov · 2020-08-05T16:34:16Z

/retest

yaacov · 2020-08-06T04:01:45Z

/retest

yaacov · 2020-08-06T07:57:56Z

@suomiy please re-review

atiratree · 2020-08-06T09:03:25Z

...rt-plugin/src/components/create-vm-wizard/redux/state-update/vm-settings-tab-state-update.ts

@@ -153,22 +153,23 @@ const osUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {
  if (iGetCommonData(state, id, VMWizardProps.isProviderImport)) {
    return;
  }
+  if (iGetCommonData(state, id, VMWizardProps.isProviderImport)) {


wanted to be extra sure :-) , removed 👍

atiratree · 2020-08-06T09:15:32Z

...rt-plugin/src/components/create-vm-wizard/redux/state-update/vm-settings-tab-state-update.ts

@@ -211,6 +212,31 @@ const baseImageUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions)
  );
 };

+const windowsToolsUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {


looks good, but we should move this updater to storage-tab-state-update.ts so the relevant will be together

moved to storage 👍

atiratree · 2020-08-06T09:18:20Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

+          </FormField>
+        </FormFieldRow>
+        {(cloneBaseDiskImage || mountWindowsGuestTools) && (
+          <Text className="kv-create-vm__input-checkbox">


i know it works, but the name does not reflect the reality :)

either

make more generic name

make 2nd class

changed to kv-create-vm__input-text-help-msg 👍

any reason you can't use pf-c-form__helper-text here instead?

atiratree · 2020-08-06T09:20:17Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

+        </FormFieldRow>
+        {(cloneBaseDiskImage || mountWindowsGuestTools) && (
+          <Text className="kv-create-vm__input-checkbox">
+            View the mounted disk in the{' '}


we probably should pluralize this depending on the number of checkboxes checked

pluralized 🥞

atiratree · 2020-08-06T09:20:56Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

+            />
+          </FormField>
+        </FormFieldRow>
+        {(cloneBaseDiskImage || mountWindowsGuestTools) && (


not having the Virtualized HW step simplifies things a bit :)

atiratree · 2020-08-06T15:16:52Z

...bevirt-plugin/src/components/create-vm-wizard/redux/state-update/storage-tab-state-update.ts

@@ -137,7 +145,34 @@ export const internalStorageDiskBusUpdater = ({
  }
 };

+const windowsToolsUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {


it should be declared/called before the internalStorageDiskBusUpdater so the validation to it applies as well

moved to before internalStorageDiskBusUpdater

please move the code block as well. We are striving for our updaters to be declared in the same order they are executed in. It is much easier to read the file from top to the bottom to asses what is happening in sequence, than jumping across different sections of the file.

There are few exceptions (eg prefillVmTemplateUpdater) where the updater would get too long and such updaters are moved to new files.

atiratree · 2020-08-06T15:19:22Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

@@ -145,6 +148,22 @@ export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
    );
    const baseImage = operatingSystemBaseImages.find((image) => image.id === os);

+    const numOfMountedDisks = cloneBaseDiskImage + mountWindowsGuestTools; // using boolean addition operator to count true


wow, this seems so hacky :)

atiratree · 2020-08-06T15:22:10Z

.../packages/kubevirt-plugin/src/components/create-vm-wizard/tabs/vm-settings-tab/os-flavor.tsx

+    const numOfMountedDisks = cloneBaseDiskImage + mountWindowsGuestTools; // using boolean addition operator to count true
+    const mountedDisksHelpMsg = numOfMountedDisks > 0 && (
+      <Text className="kv-create-vm__input-text-help-msg">
+        View the mounted {pluralize(numOfMountedDisks, 'disk', 'disks', false)} in the{' '}


we also have pluralize function in our package which is easier to use and does not depend on details-page file

using our own pluralize :-)

yaacov · 2020-08-07T08:00:03Z

/retest

yaacov · 2020-08-07T11:23:28Z

/retest

atiratree

one last thing :)

atiratree · 2020-08-07T11:46:42Z

...bevirt-plugin/src/components/create-vm-wizard/redux/state-update/storage-tab-state-update.ts

@@ -137,7 +145,34 @@ export const internalStorageDiskBusUpdater = ({
  }
 };

+const windowsToolsUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {


please move the code block as well. We are striving for our updaters to be declared in the same order they are executed in. It is much easier to read the file from top to the bottom to asses what is happening in sequence, than jumping across different sections of the file.

There are few exceptions (eg prefillVmTemplateUpdater) where the updater would get too long and such updaters are moved to new files.

yaacov · 2020-08-07T12:19:54Z

/retest

atiratree · 2020-08-07T13:42:12Z

/lgtm

openshift-ci-robot · 2020-08-07T13:42:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: suomiy, yaacov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/packages/kubevirt-plugin/OWNERS~~ [suomiy,yaacov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

yaacov · 2020-08-07T15:36:56Z

/retest

openshift-bot · 2020-08-07T15:50:43Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-08-08T00:54:45Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-08-08T02:51:45Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-08-08T03:17:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-08-08T10:26:43Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-08-08T11:49:43Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-08-08T16:32:33Z

@yaacov: All pull requests linked via external trackers have merged: openshift/console#5490. Bugzilla bug 1829542 has been moved to the MODIFIED state.

In response to this:

Bug 1829542: Add mount guest agent tools to vm wizard

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label May 19, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label May 19, 2020

openshift-ci-robot requested review from mareklibra and rawagner May 19, 2020 14:13

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2020

openshift-ci-robot assigned atiratree May 19, 2020

atiratree suggested changes May 20, 2020

View reviewed changes

openshift-ci-robot added component/core Related to console core functionality component/olm Related to OLM and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 20, 2020

yaacov force-pushed the add-mount-guest-agent-tools-to-vm-wizard branch from 36265a9 to 4eda5b6 Compare May 20, 2020 16:45

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 20, 2020

yaacov force-pushed the add-mount-guest-agent-tools-to-vm-wizard branch 2 times, most recently from 24de709 to fff5e8e Compare May 20, 2020 17:43

atiratree reviewed May 21, 2020

View reviewed changes

yaacov force-pushed the add-mount-guest-agent-tools-to-vm-wizard branch from 0dad952 to 44edcf9 Compare May 27, 2020 14:22

openshift-ci-robot removed the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label May 27, 2020

atiratree suggested changes Aug 6, 2020

View reviewed changes

yaacov added 4 commits August 6, 2020 15:34

use pluralize for number of auto generated disks

2ec411c

rename text calss from checkbox to text

6750c7a

remove extra chcek

37cd478

move win tools updater to storage

9341363

atiratree reviewed Aug 6, 2020

View reviewed changes

yaacov added 2 commits August 6, 2020 18:28

call wintools before storagebus updater

eac892b

use different pluralize method

7ab0ad2

atiratree reviewed Aug 7, 2020

View reviewed changes

move updating magic to match its invocation

51130d4

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2020

openshift-merge-robot merged commit 3069ceb into openshift:master Aug 8, 2020

spadgett added this to the v4.6 milestone Aug 11, 2020

		@@ -147,19 +147,27 @@ const flavorUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) =>

		const osUpdater = ({ id, prevState, dispatch, getState }: UpdateOptions) => {

	isHidden: asHidden(true),
	isHidden: asHidden(true, VMSettingsField.OPERATING_SYSTEM),

Bug 1829542: Add mount guest agent tools to vm wizard #5490

Bug 1829542: Add mount guest agent tools to vm wizard #5490

Conversation

yaacov commented May 19, 2020 • edited

openshift-ci-robot commented May 19, 2020

yaacov commented May 19, 2020

yaacov commented May 19, 2020

atiratree commented May 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atiratree May 21, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcbailey commented May 21, 2020

This comment was marked as outdated.

Choose a reason for hiding this comment

matthewcarleton commented May 21, 2020

matthewcarleton commented May 21, 2020

atiratree commented May 22, 2020 • edited

matthewcarleton commented May 22, 2020

atiratree commented May 22, 2020

pcbailey commented May 26, 2020

openshift-ci-robot commented May 27, 2020

openshift-ci-robot commented Aug 5, 2020

yaacov commented Aug 5, 2020

yaacov commented Aug 6, 2020

yaacov commented Aug 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaacov commented Aug 7, 2020

yaacov commented Aug 7, 2020

atiratree left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaacov commented May 19, 2020 •

edited

atiratree May 21, 2020 •

edited

atiratree commented May 22, 2020 •

edited