search

rnelson0 / vCenter-roles

Set up roles for common applications to access vCenter
Apache License 2.0
11 stars 3 forks source link

Update Import-VIRole.ps1 #10

Closed Zsoldier closed 8 years ago

Zsoldier commented 8 years ago

Added Overwrite Option.

rnelson0 commented 8 years ago

@Zsoldier this sounds like a great idea. If you can fix the numbering and indentation level, I'll merge it. Thanks!

Zsoldier commented 8 years ago

I fixed the numbering, but am unclear on your indentation requests.

rnelson0 commented 8 years ago

I made some more specific comments. I'm not sure what you're editing in, it could be a hard vs soft tab issue. Maybe open it in notepad to see how it looks there?

Zsoldier commented 8 years ago

I'm just editing from the github web editor. Maybe different when doing the edits from a Mac?

rnelson0 commented 8 years ago

@Zsoldier For easier view, here's a link to imgur.

You can see that line 109, 124-125, 129-131, 136 need an extra space. 133-135 need 4 extra spaces. Hope the visual helps.

Zsoldier commented 8 years ago

You could just merge and remove them yourself. Honestly the extra spaces/tabs don't matter in Powershell. It matters more in a language like Python. Also, what you are pointing out doesn't show up in my view on the web so I can't really correct something I don't see.

On Tue, Feb 23, 2016 at 11:14 AM, Rob Nelson notifications@github.com wrote:

@Zsoldier https://github.com/Zsoldier For easier view, here's a link to imgur http://imgur.com/nFs2QLp.

You can see that line 109, 124-125, 129-131, 136 need an extra space. 133-135 need 4 extra spaces. Hope the visual helps.

— Reply to this email directly or view it on GitHub https://github.com/rnelson0/vCenter-roles/pull/10#issuecomment-187768794 .