My understanding from go.work is that it should no longer be needed to add those replace commands
Is that actually not the case?

I actually couldn't get go mod tidy to play nice without them. I can mess around with it a bit more but figured this was fine for now

I was able to drop the replace from ./envoy but that one still complained so I left it in.

Maybe this is an indication that we don't have the right structure? Since the "envoy" and "contrib" protobufs are bound to the same Envoy release, does it make sense to allow the possibility for a program to import different versions of them?

How hard would it be to move the contrib protobufs to "envoy/contrib"?

And I suppose that a similar question arises for the "./ratelimit" directory.

heres a branch of Contour using the above branch: https://github.com/sunjayBhatia/contour/tree/use-go-control-plane-multi-module

(the above go-control-plane branch also copies the ratelimit module to a package in the new api module)

so this branch of Contour works with this PR's branch (plus this commit sunjayBhatia@4be6f62): sunjayBhatia/contour@dc8fa81

i think either approach will work just fine, though the upgrade/transition UX might be cleaner (no possibility for users complaining about ambiguous package paths if they are requiring mismatched versions) if the new modules have a new package path

(the above go-control-plane branch also copies the ratelimit module to a package in the new api module)

I'll check later, but me recollection is that the ratelimit protos don't come from the envoy repository. So they probably need to be versioned differently and it might not make sense for them to be in the same module as the envoy api.

JFYI, we should be prepared for some pushback on this. It's possible that some consumer of this module will be using earlier Go versions (see #271 for example).

I can try and roll back to 1.18 but that is a bare minimum requirement for workspaces. Wondering if we should see if it's that problematic and roll back if people complain.

+1 please don't use 1.20. In grpc-go, which uses this, we support 1.18+ and also haven't made any changes AFAIK that would prevent even 1.17 or maybe 1.16 from working (e.g. no use of any or generics). IMO this (and the use of new language features) should stay at the oldest version you can reasonably handle supporting.

@alecholmez it sounds like this is the most contentious point within this change set.

Sorry for the dumb question...what do go workspaces actually do for you? Someone wanted to add a go.work file to our repo, but I didn't fully understand how it would help. We have several modules and don't use a workspace, and things generally work fine.

I think they are more useful in local development etc. when you have modules side-by-side rather than a module hierarchy like we have here, you should mainly be able to remove the replace directives but it doesn't seem (from my playing around with it as well) that we get that here because of the package/module structure

e.g. to get the other modules in to actually use the toher modules in this repo all the way, had to do this: sunjayBhatia@4be6f62

so I'm not sure how much the workspace is getting us

Is the decision to ultimately remove the workspace? I'm fine with that

Since this was discussed before but not resolved, and I see this PR was approved:

https://go.dev/ref/mod#go-work-file

It is generally inadvisable to commit go.work files into version control systems, for two reasons:

• A checked-in go.work file might override a developer’s own go.work file from a parent directory, causing confusion when their use directives don’t apply.
• A checked-in go.work file may cause a continuous integration (CI) system to select and thus test the wrong versions of a module’s dependencies. CI systems should generally not be allowed to use the go.work file so that they can test the behavior of the module as it would be used when required by other modules, where a go.work file within the module has no effect.

That said, there are some cases where committing a go.work file makes sense. For example, when the modules in a repository are developed exclusively with each other but not together with external modules, there may not be a reason the developer would want to use a different combination of modules in a workspace. In that case, the module author should ensure the individual modules are tested and released properly.

And the related issue that ultimately led to this addition:

golang/go#53502

I don’t mind seeing them removed.
By the way, help on multimodule release is welcome here