Update Environment and VM attributes

[lfirwin]

Changes made to update the following:

• Optionally add variable number of disks to a VM
• Optionally give an environment a name
• Optionally assign new environment to a project
• Updated README.md and CHANGELOG.md to reflect changes.
• Updated version to 0.3.7

[spazquest]

Thank you very much for this PR. Unfortunately we need some changes before we can accept it.

1. The semantics for the disks setting will make it harder to support other use cases cleanly in the future. I have a proposed fix for this below.
2. Are the Gemfile and Gemfile.lock changes necessary? I'm concerned about backward compatibility. Please remove if not necessary.
3. For tracking purposes, we would prefer separate pull requests for unrelated changes. (You can leave the environment name and project id changes in the same patch if you want, but ideally the project id change would have its own issue created and separate PR.)
4. We don't normally accept PRs from outside parties that update the plugin version or modify the changelog.

Regarding the disks setting: I don't think adding should be the default behavior, since we will support resizing and deletion at some point. Coming up with the right format for that could be a bit tricky, and it will be helpful to start with a clean slate. Also, adding disks every time Vagrant starts a VM should not be the default behavior, since this could be very surprising.

Here's a possible fix to get you folks up and running. I propose that we use a specific format for the disks setting that handles your use case exactly the way you have it here. If it doesn't match this format, we just ignore the setting for now. Once we work out the details for those other use cases, we would support this as a legacy format.

box.disks = :add_on_run, [<disk size>, ...]

How does that sound?

P.S. I'll write up some contributor guidelines as soon as I get a chance.

[lfirwin]

Feel free to decline the PR and take the pieces of code you want out of it. Your points are completely valid. I'll try your fix for disks and see how that works. I've created a .gem file and we're using it as our vagrant-skytap plugin for now. When you make the appropriate updates we'll replace the plugin with the official one. And while you are in the code, I'd ask that you also look at adding an enhancement to update VM attributes, such as the display name. Yeah, it's cosmetic, but it's helpful to quickly distinguish between VMs in the Skytap console. I'll add an issue in the issues list for this request.

…

On Fri, Mar 23, 2018 at 5:53 PM, Eric True ***@***.***> wrote: Thank you very much for this PR. Unfortunately we need some changes before we can accept it. 1. The semantics for the disks setting will make it harder to support other use cases cleanly in the future. I have a proposed fix for this below. 2. Are the Gemfile and Gemfile.lock changes necessary? I'm concerned about backward compatibility. Please remove if not necessary. 3. For tracking purposes, we would prefer separate pull requests for unrelated changes. (You can leave the environment name and project id changes in the same patch if you want, but ideally the project id change would have its own issue created and separate PR.) 4. We don't normally accept PRs from outside parties that update the plugin version or modify the changelog. Regarding the disks setting: I don't think adding should be the default behavior, since we will support resizing and deletion at some point. Coming up with the right format for that could be a bit tricky, and it will be helpful to start with a clean slate. Also, adding disks every time Vagrant starts a VM should not be the default behavior, since this could be very surprising. Here's a possible fix to get you folks up and running. I propose that we use a specific format for the disks setting that handles your use case exactly the way you have it here. If it doesn't match this format, we just ignore the setting for now. Once we work out the details for those other use cases, we would support this as a legacy format. box.disks = :add_on_run, [<disk size>, ...] How does that sound? P.S. I'll write up some contributor guidelines as soon as I get a chance. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#30 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATdw5Yt3Znh0QBo7x_nSx3O9nx5bZtzvks5thYsAgaJpZM4S2MtE> .