search

red-hat-storage / ocs-operator

Operator for RHOCS
Apache License 2.0
85 stars 184 forks source link

provider: split protobuf related pkgs into a separate go module #2798

Closed leelavg closed 2 months ago

leelavg commented 2 months ago

pb -> now contains generated proto stubs, interface implementations and client api

existing package paths are updated accordingly, this is to solve huge dep updates (whole of ocs-operator) being pulled when a change is done to proto and corresponding stubs.

leelavg commented 2 months ago

atm client-op doesn't have dependency on ocs-op expect for newly created standalone modules

> go mod why github.com/red-hat-storage/ocs-operator/v4
# github.com/red-hat-storage/ocs-operator/v4
(main module does not need package github.com/red-hat-storage/ocs-operator/v4)

> go mod why github.com/red-hat-storage/ocs-operator/v4/services/provider/client
# github.com/red-hat-storage/ocs-operator/v4/services/provider/client
github.com/red-hat-storage/ocs-client-operator/internal/controller
github.com/red-hat-storage/ocs-operator/v4/services/provider/client

> go mod why github.com/red-hat-storage/ocs-operator/v4/services/provider/interfaces
# github.com/red-hat-storage/ocs-operator/v4/services/provider/interfaces
github.com/red-hat-storage/ocs-client-operator/service/status-report
github.com/red-hat-storage/ocs-operator/v4/services/provider/interfaces

> go mod why github.com/red-hat-storage/ocs-operator/v4/services/provider/pb
# github.com/red-hat-storage/ocs-operator/v4/services/provider/pb
github.com/red-hat-storage/ocs-client-operator/internal/controller
github.com/red-hat-storage/ocs-operator/v4/services/provider/client
github.com/red-hat-storage/ocs-operator/v4/services/provider/pb
leelavg commented 2 months ago

can review only first commit and second is the result of make deps-update

leelavg commented 2 months ago

/hold

Checking client-op compilation based on this PR

nb-ohad commented 2 months ago

I don't think we should have a package with only the PB. Let have an offline discussion on this

leelavg commented 2 months ago

I don't think we should have a package with only the PB. Let have an offline discussion on this

  • updated, pls review.
leelavg commented 2 months ago

/unhold

Tested below scenarios:

  1. updating the proto and make target regenerates the stubs
  2. used this pr as dep in client-op and able to compile the binaries
leelavg commented 2 months ago

I think it makes more sense calling the new package provider/api as it contain more then just protobufs

  • done.
nb-ohad commented 2 months ago

/lgtm

openshift-ci[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, nb-ohad

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](https://github.com/red-hat-storage/ocs-operator/blob/main/OWNERS)~~ [nb-ohad] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment