copy/paste instructions for warden

cloudfoundry-attic / etcd-release

Apache License 2.0

3 stars 17 forks source link

copy/paste instructions for warden #5

Closed drnic closed 8 years ago

cfdreddbot commented 9 years ago

Hey drnic!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

cf-gitbot commented 9 years ago

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/106798546.

Amit-PivotalLabs commented 9 years ago

Hi @drnic

It's hard to tell what content changes you've made due to all the whitespace changes. What does it have to do with warden? Also, have you taken a look at the README preview with these new whitespace changes? I suspect the new look is unintentional:

drnic commented 9 years ago

Sorry will fix.

On Tue, Oct 27, 2015 at 3:26 PM, Amit Gupta notifications@github.com wrote:

Hi @drnic It's hard to tell what content changes you've made due to all the whitespace changes. What does it have to do with warden? Also, have you taken a look at the README preview with these new whitespace changes? I suspect the new look is unintentional:

Reply to this email directly or view it on GitHub: https://github.com/cloudfoundry-incubator/etcd-release/pull/5#issuecomment-151663603

drnic commented 9 years ago

@Amit-PivotalLabs fixed the PR; sorry about that.

Amit-PivotalLabs commented 9 years ago

Thanks, I've prioritized this PR and added the following:

README should mention the required property-overrides stub in the “Generating deployment manifest” section
README should mention that we provide a default property-overrides stub for BOSH-Lite purposes (as we do for the other required stubs)
generate_etcd_deployment_manifest provides an example in the Usage where the property-overrides and iaas-settings are out of order; both orders are effectively equivalent, but is confusing, and the order in the example should be fixed: https://github.com/cloudfoundry-incubator/etcd-release/blob/master/scripts/generate_etcd_deployment_manifest#L20-L21

Amit-PivotalLabs commented 9 years ago

@drnic to clarify, you don't need to make any more changes, our team will merge your PR and address the additional points I raised, which are omissions in the existing documentation, not a problem with your PR.

medvedzver commented 9 years ago

Merged, and added changes per amit's commits.

Mikhail & @robdimsdale

drnic commented 9 years ago

Thanks guys

On Tue, Nov 3, 2015 at 1:33 AM, medvedzver notifications@github.com wrote:

Merged, and added changes per amit's commits.

Mikhail & @robdimsdale

Reply to this email directly or view it on GitHub: https://github.com/cloudfoundry-incubator/etcd-release/pull/5#issuecomment-153203830

Amit-PivotalLabs commented 8 years ago

This was merged separately, closing PR.