Skip to content

Conversation

@maxwelldb
Copy link
Contributor

@maxwelldb maxwelldb added this to the Future Release milestone May 15, 2020
@maxwelldb maxwelldb requested a review from iamemilio May 15, 2020 17:54
@maxwelldb maxwelldb self-assigned this May 15, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 15, 2020
@maxwelldb
Copy link
Contributor Author

At the moment, this PR just adds the relevant param w/a commented question. Unsure about additional work required at this point.

@maxwelldb maxwelldb force-pushed the osp-customer-subnets-osdocs1023 branch from f3e32e1 to 4d8f477 Compare May 20, 2020 13:49
@openshift-docs-preview-bot

The preview will be available shortly at:

@maxwelldb
Copy link
Contributor Author

Relevant: openshift/installer#3647

@maxwelldb
Copy link
Contributor Author

Also relevant: openshift/installer@fb30018

@maxwelldb maxwelldb force-pushed the osp-customer-subnets-osdocs1023 branch from 0130b79 to 69453f8 Compare June 22, 2020 22:31
@maxwelldb
Copy link
Contributor Author

maxwelldb commented Jun 22, 2020

@iamemilio I chunked off the content into its own module--it was getting a bit much for the properties table.

Questions:

  1. +1/-1?
  2. How frequently are customers expected to use this feature? All of the time? Half of all deployments? Infrequently? I'm not sure if I should stash the new module away in on its own page or if it should be included in the flow of all installation docs.

@maxwelldb maxwelldb marked this pull request as ready for review June 22, 2020 22:33
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 22, 2020
@maxwelldb maxwelldb requested a review from iamemilio June 22, 2020 22:33
@maxwelldb maxwelldb force-pushed the osp-customer-subnets-osdocs1023 branch 2 times, most recently from b4ef0fe to 1c1c447 Compare June 22, 2020 22:35
@maxwelldb maxwelldb force-pushed the osp-customer-subnets-osdocs1023 branch from 1c1c447 to c53757e Compare June 22, 2020 22:44
@iamemilio
Copy link

lgtm

@maxwelldb maxwelldb removed the request for review from iamemilio June 25, 2020 19:15
@maxwelldb maxwelldb force-pushed the osp-customer-subnets-osdocs1023 branch 2 times, most recently from 3dd7573 to 557104b Compare June 25, 2020 19:36
@maxwelldb maxwelldb force-pushed the osp-customer-subnets-osdocs1023 branch from 557104b to 3663f6a Compare June 25, 2020 19:39
@maxwelldb
Copy link
Contributor Author

@maxwelldb maxwelldb requested a review from morenod July 1, 2020 13:11
@maxwelldb maxwelldb requested review from a user and gpei July 1, 2020 13:11
Copy link

@morenod morenod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@maxwelldb
Copy link
Contributor Author

@morenod Thanks!

@maxwelldb maxwelldb added the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 2, 2020
@bobfuru bobfuru self-requested a review July 2, 2020 13:45
Copy link
Contributor

@bobfuru bobfuru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small nits; otherwise, LGTM.

@maxwelldb maxwelldb requested review from bobfuru and removed request for a user and gpei July 2, 2020 14:38
@bobfuru bobfuru added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jul 2, 2020
Copy link
Contributor

@bobfuru bobfuru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@maxwelldb
Copy link
Contributor Author

@bobfuru Thanks!

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 2, 2020
@maxwelldb maxwelldb merged commit 6b580b2 into openshift:master Jul 2, 2020
@maxwelldb
Copy link
Contributor Author

/cherry-pick enterprise-4.5

@openshift-cherrypick-robot

@maxwelldb: new pull request created: #23427

Details

In response to this:

/cherry-pick enterprise-4.5

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.

@maxwelldb
Copy link
Contributor Author

@vikram-redhat I forgot to squash this pre-merge. 👎 What's the preferred strategy for fixing this in master--force push with lease, leave it and don't do it again, or something else?

@maxwelldb maxwelldb restored the osp-customer-subnets-osdocs1023 branch July 2, 2020 16:18
@bobfuru
Copy link
Contributor

bobfuru commented Jul 2, 2020

@vikram-redhat I forgot to squash this pre-merge. 👎 What's the preferred strategy for fixing this in master--force push with lease, leave it and don't do it again, or something else?

Sorry, I noticed the commits after my LGTM but didn't mention it because I thought it was primarily a practice we use to make it easier for reviewers to follow updates. Curious to hear if there are other reasons as well.

@maxwelldb
Copy link
Contributor Author

@bobfuru Yeah, I've always avoided squashing until the last second to (maybe) make reviewing easier. My goof.

@vikram-redhat
Copy link
Contributor

@vikram-redhat I forgot to squash this pre-merge. 👎 What's the preferred strategy for fixing this in master--force push with lease, leave it and don't do it again, or something else?

The second last option.. leave it and let's not do it again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.5 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants