Skip to content

Conversation

@RichardHoch
Copy link
Contributor

@RichardHoch RichardHoch commented Jul 3, 2022

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 3, 2022
@RichardHoch RichardHoch marked this pull request as ready for review July 6, 2022 08:52
@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 6, 2022
@RichardHoch RichardHoch force-pushed the velero_plugin_congig branch 2 times, most recently from 457dfd3 to fd8e5d3 Compare July 6, 2022 09:59

Choose a reason for hiding this comment

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

I'm not sure all of this is suppose to be on a "procedure".
I think it's better to just specify the different plugin types as options, like in the upstream doc:
https://github.com/openshift/oadp-operator/blob/master/docs/config/plugins.md

It may cause a confusion and the steps make it look like custom plguin is dependent on default plugins existence...

Copy link
Contributor Author

@RichardHoch RichardHoch Jul 6, 2022

Choose a reason for hiding this comment

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

@mperetzred Should I include the code sample as an example, like in the upstream documentation, without making it into a procedure?

Choose a reason for hiding this comment

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

IMO, yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

Copy link
Member

Choose a reason for hiding this comment

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

I would add openshift plugin in the examples by default. Here and also in other modules.

@RichardHoch RichardHoch force-pushed the velero_plugin_congig branch from 1948aa2 to 697d047 Compare July 6, 2022 15:59
@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 6, 2022
@RichardHoch RichardHoch force-pushed the velero_plugin_congig branch from 3b743f9 to ee618d4 Compare July 6, 2022 16:04
@RichardHoch
Copy link
Contributor Author

@kaovilai and @mperetzred : i rewrote this as an "information" section, instead of as a procedure. Please review.

Choose a reason for hiding this comment

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

just remove this + character

Choose a reason for hiding this comment

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

just remove this + character

@mperetzred
Copy link

there is some redundant +, added comments where. Otherwise lgtm

@RichardHoch RichardHoch force-pushed the velero_plugin_congig branch from ee618d4 to 7ac346e Compare July 7, 2022 11:46
@RichardHoch RichardHoch requested a review from mperetzred July 7, 2022 12:01
@RichardHoch
Copy link
Contributor Author

@mperetzred -- removed extra "+" signs.

@RichardHoch
Copy link
Contributor Author

@sbeskin-redhat Please review this PR. As you can see from it and the discussions in the Jira, it is a CONCEPT module, not a PROCEDURE.

@RichardHoch RichardHoch force-pushed the velero_plugin_congig branch 2 times, most recently from c026d6b to 3c962dd Compare July 10, 2022 12:13
@bergerhoffer
Copy link
Contributor

The enterprise-4.12 label has been added to this PR.

This is because your PR targets the main branch and is labeled for enterprise-4.11. And any PR going into main must also target the latest version branch (enterprise-4.12).

If the update in your PR does NOT apply to version 4.12 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main.

@stevsmit
Copy link
Member

stevsmit commented Aug 31, 2022

Merge review approved. This review is considered "crucial" for the OADP release. This pull request can be merged upon OADP GA without the need for a second merge review.

@mburke5678 mburke5678 merged commit 43f84a2 into openshift:main Sep 1, 2022
@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.6

@openshift-cherrypick-robot

@mburke5678: new pull request created: #49884

In response to this:

/cherrypick enterprise-4.6

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.

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.7

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.8

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.9

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.10

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.11

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.12

@openshift-cherrypick-robot

@mburke5678: new pull request created: #49885

In response to this:

/cherrypick enterprise-4.7

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.

@openshift-cherrypick-robot

@mburke5678: new pull request created: #49886

In response to this:

/cherrypick enterprise-4.8

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.

@openshift-cherrypick-robot

@mburke5678: new pull request created: #49887

In response to this:

/cherrypick enterprise-4.9

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.

@openshift-cherrypick-robot

@mburke5678: new pull request created: #49888

In response to this:

/cherrypick enterprise-4.10

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.

@openshift-cherrypick-robot

@mburke5678: new pull request created: #49889

In response to this:

/cherrypick enterprise-4.11

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.

@openshift-cherrypick-robot

@mburke5678: new pull request created: #49890

In response to this:

/cherrypick enterprise-4.12

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.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants