Revert "Convert client.List to use functional options"

[mengqiy]

Reverts #106

#106 is a breaking change to the client interface.
Per suggestion from @pwittrock, we are going to revert it for now before we make a decision about how to deal with it.
We can decide later if we want to introduce the new signature as a new method and mark the old one as deprecated or do a v0.2.0 release for it.

[mengqiy]

cc: @grantr

[droot]

@grantr @DirectXMan12 This introduces breaking change without any deprecation (which is making harder to make it available in v0.1.x versions of controller-runtime). We need to cut a release to unblock some other features, lets revert it for now, and figure out how we want to roll out this breaking change.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: mengqiy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[grantr]

@droot has the situation changed since #106 (comment)?

we have sorted out the versioning story for controller-runtime/tools and Kubebuilder now uses versioned pkgs of controller-*, so we can move forward with these changes now.

FWIW #94 is technically a breaking change to the interface also, though unlike #106 it adds critical functionality to client.Delete.

[mengqiy]

FWIW #94 is technically a breaking change to the interface also, though unlike #106 it adds critical functionality to client.Delete.

@grantr we can have 0 DeleteOptionFunc, so #94 is not a breaking change.

[droot]

@grantr Though we have started versioning controller-runtime, we still do not have a process for managing multiple revisions like branches for 0.1.x and 0.2.x.. and cherrypicking etc. I was assuming we will introduce "ListVariadic" version without dropping the existing "List" method which is not breaking compatibility (in hard way) because interface and implementation are both the provided by controller-runtime. So if your code uses client interface and same implementation, update should work. And then in major version (0.2.0), we remove rename ListVariadic to List.

Sorry, I was OOO last week and couldn't participate in review.

[grantr]

we can have 0 DeleteOptionFunc, so #94 is not a breaking change.

It is a breaking interface change. Any existing implementations of the interface will no longer compile. See #94 (comment).

[mengqiy]

usage of the client interface (Delete() calls) is backward compatible, but implementation of the client interface is not.

Yes, this is true. From this perspective, even introducing "ListVariadic" version without dropping the existing "List" method is still a breaking change.
But IMO not breaking the usages of our client implementation is more important than not breaking user's implementation. I assume there are much more people using our client implementation than implementing their own client.

[grantr]

That seems like a reasonable policy to me. Maybe something to add to the compatibility guarantee, if there is one.

[DirectXMan12]

I think we should have a broader conversation about this... things like ListVariadic are a nightmare from a user perspective. I always hate using libraries like that, and I've seen users getting very confused.