Adding VMware vSphere Disk Mode support (Dependent, Independent - P… #4314

sbernhard · 2017-02-21T12:49:12Z

fixes #18815 - Adding VMware vSphere Disk Mode support

…ersistent, Independent - Nonpersistent)

theforeman-bot · 2017-02-21T12:49:12Z

There were the following issues with the commit message:

5fbb00a must be in the format fixes #redmine_number - brief description
length of the first commit message line for 5fbb00a exceeds 65 characters
commit message for 5fbb00a is not wrapped at 72nd column

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

theforeman-bot · 2017-02-21T12:49:12Z

Can an existing organization member please verify this patch?

theforeman-bot · 2017-02-21T12:49:12Z

Can an existing organization member please verify this patch?

theforeman-bot · 2017-02-21T12:49:12Z

Can an existing organization member please verify this patch?

mention-bot · 2017-02-21T12:49:12Z

@BernhardAtix, thanks for your PR! By analyzing the history of the files in this pull request, we identified @timogoebel, @tstrachota and @domcleal to be potential reviewers.

ohadlevy · 2017-02-21T13:03:44Z

@BernhardAtix thanks! FYI - please note that #3979 potentially rewrite this part of the form

ohadlevy · 2017-02-21T13:05:45Z

ok to test

timogoebel

@BernhardAtix : Thanks for your PR. I left some comments inline. Could you also please provide a Redmine issue for this?

timogoebel · 2017-02-21T14:20:16Z

app/views/compute_resources_vms/form/vmware/_volume.html.erb

@@ -23,3 +23,17 @@
           :label => _("Eager zero"), :label_size => "col-md-2", :disabled => !new_host},
           "true",
           "false" %>
+<%
+# regarding to the following documentation:


Could you please remove these comments? They don't belong in Foreman's source.

timogoebel · 2017-02-21T14:24:46Z

app/views/compute_resources_vms/form/vmware/_volume.html.erb

+#  ['undoable', :undoable],
+#  ['append', :append]]
+%>
+<% disk_mode_values = [['Dependent', :persistent],


Please move these options to vmware's compute resource model.

firmware_types is one example for this.
https://github.com/theforeman/foreman/blob/develop/app/models/compute_resources/foreman/model/vmware.rb#L170

Please note, that you need to mark the names as translatable, e.g. _('Dependent').

timogoebel · 2017-02-21T14:25:00Z

app/views/compute_resources_vms/form/vmware/_volume.html.erb

+   ['Independent - Persistent', :independent_persistent],
+   ['Independent - Nonpersistent', :independent_nonpersistent]] %>
+<%= selectable_f f, :mode, disk_mode_values, { }, :class => "span5", :label => _("Disk mode"), :label_size => "col-md-2", :disabled => !new_host %>
+


Nitpick: Please remote the empty line.

theforeman-bot · 2017-02-21T15:11:13Z

There were the following issues with the commit message:

5fbb00a must be in the format fixes #redmine_number - brief description
length of the first commit message line for 5fbb00a exceeds 65 characters
commit message for 5fbb00a is not wrapped at 72nd column
f7d4b42 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

theforeman-bot · 2017-03-01T11:31:22Z

There were the following issues with the commit message:

5fbb00a must be in the format fixes #redmine_number - brief description
length of the first commit message line for 5fbb00a exceeds 65 characters
commit message for 5fbb00a is not wrapped at 72nd column
f7d4b42 must be in the format fixes #redmine_number - brief description
bfd187e must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

theforeman-bot · 2017-03-06T14:30:28Z

There were the following issues with the commit message:

5fbb00a must be in the format fixes #redmine_number - brief description
length of the first commit message line for 5fbb00a exceeds 65 characters
commit message for 5fbb00a is not wrapped at 72nd column
f7d4b42 must be in the format fixes #redmine_number - brief description
bfd187e must be in the format fixes #redmine_number - brief description
3e16400 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

theforeman-bot · 2017-03-06T15:37:43Z

There were the following issues with the commit message:

5fbb00a must be in the format fixes #redmine_number - brief description
length of the first commit message line for 5fbb00a exceeds 65 characters
commit message for 5fbb00a is not wrapped at 72nd column
f7d4b42 must be in the format fixes #redmine_number - brief description
bfd187e must be in the format fixes #redmine_number - brief description
3e16400 must be in the format fixes #redmine_number - brief description
f448cab must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

sbernhard · 2017-03-06T21:47:36Z

fixes #18815
=> added redmine issue for it

timogoebel

@BernhardAtix : Thanks for the updates. I left one small comment inline.

timogoebel · 2017-03-07T17:12:10Z

app/models/compute_resources/foreman/model/vmware.rb

@@ -175,6 +175,14 @@ def firmware_types
      }
    end

+    def disk_mode_types
+      {
+        "persistent" => N_("Persistent"),


This isn't translated correctly.
N_() just marks a string for translation but doesn't actually translate it.
Please use either _() or translate it in the view.

<%= select_f f, :mode, compute_resource.disk_mode_types, :first, ->(d) { _(d.last) }, { }, :class => "span5", :label => _("Disk mode"), :label_size => "col-md-2", :disabled => !new_host %>

theforeman-bot · 2017-03-07T20:23:33Z

There were the following issues with the commit message:

5fbb00a must be in the format fixes #redmine_number - brief description
length of the first commit message line for 5fbb00a exceeds 65 characters
commit message for 5fbb00a is not wrapped at 72nd column
f7d4b42 must be in the format fixes #redmine_number - brief description
bfd187e must be in the format fixes #redmine_number - brief description
3e16400 must be in the format fixes #redmine_number - brief description
f448cab must be in the format fixes #redmine_number - brief description
1eeb78f must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

timogoebel

Thanks @BernhardAtix. Works well.

- Adding VMware vSphere Disk Mode support (Dependent, Independent - P…

5fbb00a

…ersistent, Independent - Nonpersistent)

theforeman-bot added Waiting on contributor Needs testing Not yet reviewed labels Feb 21, 2017

theforeman-bot removed the Not yet reviewed label Feb 21, 2017

timogoebel requested changes Feb 21, 2017

View reviewed changes

theforeman-bot added the Waiting on contributor label Feb 21, 2017

Remove comments as suggested

f7d4b42

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Feb 21, 2017

theforeman-bot added the Waiting on contributor label Feb 21, 2017

- remove empty line

bfd187e

theforeman-bot added Needs re-review Needs testing Waiting on contributor and removed Waiting on contributor labels Mar 1, 2017

Merge remote-tracking branch 'upstream/develop' into develop

3e16400

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Mar 6, 2017

theforeman-bot added the Waiting on contributor label Mar 6, 2017

- Fix issue requested by code review

f448cab

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Mar 6, 2017

theforeman-bot added the Waiting on contributor label Mar 6, 2017

timogoebel requested changes Mar 7, 2017

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels Mar 7, 2017

Fix translation issue

1eeb78f

theforeman-bot added Needs re-review Needs testing Waiting on contributor and removed Waiting on contributor labels Mar 7, 2017

timogoebel approved these changes Mar 8, 2017

View reviewed changes

timogoebel added Ready to merge and removed Needs re-review Needs testing Waiting on contributor labels Mar 8, 2017

timogoebel merged commit 1d5f6c2 into theforeman:develop Mar 8, 2017

timogoebel mentioned this pull request Mar 20, 2017

Fixes #4509 - VMWare scsi react #4366

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding VMware vSphere Disk Mode support (Dependent, Independent - P… #4314

Adding VMware vSphere Disk Mode support (Dependent, Independent - P… #4314

sbernhard commented Feb 21, 2017 •

edited

Loading

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Feb 21, 2017

mention-bot commented Feb 21, 2017

ohadlevy commented Feb 21, 2017

ohadlevy commented Feb 21, 2017

timogoebel left a comment •

edited

Loading

timogoebel Feb 21, 2017

timogoebel Feb 21, 2017

timogoebel Feb 21, 2017

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Mar 1, 2017

theforeman-bot commented Mar 6, 2017

theforeman-bot commented Mar 6, 2017

sbernhard commented Mar 6, 2017

timogoebel left a comment

timogoebel Mar 7, 2017

theforeman-bot commented Mar 7, 2017

timogoebel left a comment

Adding VMware vSphere Disk Mode support (Dependent, Independent - P… #4314

Adding VMware vSphere Disk Mode support (Dependent, Independent - P… #4314

Conversation

sbernhard commented Feb 21, 2017 • edited Loading

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Feb 21, 2017

mention-bot commented Feb 21, 2017

ohadlevy commented Feb 21, 2017

ohadlevy commented Feb 21, 2017

timogoebel left a comment • edited Loading

Choose a reason for hiding this comment

timogoebel Feb 21, 2017

Choose a reason for hiding this comment

timogoebel Feb 21, 2017

Choose a reason for hiding this comment

timogoebel Feb 21, 2017

Choose a reason for hiding this comment

theforeman-bot commented Feb 21, 2017

theforeman-bot commented Mar 1, 2017

theforeman-bot commented Mar 6, 2017

theforeman-bot commented Mar 6, 2017

sbernhard commented Mar 6, 2017

timogoebel left a comment

Choose a reason for hiding this comment

timogoebel Mar 7, 2017

Choose a reason for hiding this comment

theforeman-bot commented Mar 7, 2017

timogoebel left a comment

Choose a reason for hiding this comment

sbernhard commented Feb 21, 2017 •

edited

Loading

timogoebel left a comment •

edited

Loading