Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add volumegroupreplicationclass controller code #749

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add volumegroupreplicationclass controller code #749

wants to merge 2 commits into from
+77 −2

Conversation

Nikhil-Ladha
Copy link
Contributor

@Nikhil-Ladha Nikhil-Ladha commented Jan 16, 2025 •
edited
Loading

This PR adds volumegroupreplicationclass controller code and also docs for the vgrclass CR.
This is a smaller PR, part of the bigger PR #610 for easier reviews of the other PR.

@Nikhil-Ladha Nikhil-Ladha force-pushed the vgrclass branch 2 times, most recently from 3658fcf to 911383d Compare January 16, 2025 12:40
@Nikhil-Ladha
Copy link
Contributor Author

@Madhu-1 @nixpanic this one too.

Madhu-1
Madhu-1 previously approved these changes Jan 17, 2025
View reviewed changes
@Nikhil-Ladha
volumegroup: add volumegroupreplicationclass controller code
929ad18 
add volumegroupreplicationclass controller code

Signed-off-by: Nikhil-Ladha <[email protected]>
@Nikhil-Ladha
docs: add docs for volumegroupreplicationclass
74dd637 
added docs for volumegroupreplicationclass

Signed-off-by: Nikhil-Ladha <[email protected]>
@mergify mergify bot dismissed Madhu-1’s stale review January 17, 2025 09:14

Pull request has been modified.

@nixpanic
Copy link
Collaborator

I am not sure if it makes sense to merge this, There is a single unused function, which is, well obviously, unused. What is the benefit of merging this? If there is a benefit, please mention it in the commit message instead of add volumegroupreplicationclass controller code, which isn't very descriptive.

@Nikhil-Ladha
Copy link
Contributor Author

I am not sure if it makes sense to merge this, There is a single unused function, which is, well obviously, unused. What is the benefit of merging this? If there is a benefit, please mention it in the commit message instead of add volumegroupreplicationclass controller code, which isn't very descriptive.

This is just a smaller PR part of the bigger #610 PR to make the review process of the other one easier.
I have updated the PR description to mention the same.

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

@Madhu-1 Madhu-1 Madhu-1 left review comments

@black-dragon74 black-dragon74 Awaiting requested review from black-dragon74

@nixpanic nixpanic Awaiting requested review from nixpanic

@Rakshith-R Rakshith-R Awaiting requested review from Rakshith-R

@yati1998 yati1998 Awaiting requested review from yati1998

@iPraveenParihar iPraveenParihar Awaiting requested review from iPraveenParihar

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Nikhil-Ladha @nixpanic @Madhu-1 @iPraveenParihar