From f1b499f578a686aa33caf4ee5be849718355b2c4 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Tue, 23 May 2023 12:11:07 +0100 Subject: [PATCH 1/4] attempt to clarify in short the modules proliferation design Signed-off-by: Ashley Davis --- design/20230302.gomod.md | 55 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/design/20230302.gomod.md b/design/20230302.gomod.md index feeadb0aa..86afcebce 100644 --- a/design/20230302.gomod.md +++ b/design/20230302.gomod.md @@ -1,9 +1,58 @@ # Design: More Modules -NB: This design doc follows from a Hackathon by @SgtCoDFish and @inteon. The intention here is to describe what we did -and what we discovered, with an eye to seeking consensus and merging upstream. +NB: This design doc follows from a Hackathon by [@SgtCoDFish](https://github.com/SgtCoDFish) and [@inteon](https://github.com/inteon). -## Problem Statement +The intention here is to describe what we did and what we discovered, with an eye to seeking consensus and merging upstream. + +## In Short + +### Assumptions / Axioms + +- It's hard or impossible to upgrade our dependencies months after a release +- We won't change our conservative approach to backports +- Fewer dependencies in our core go module is a good thing +- It's OK if people can't import our binaries as go modules + +### Solution + +- Create a go module for each binary +- Create go modules for integration and e2e tests +- Utilise local replace statements where possible +- - i.e. Binaries have a local replace for the core cert-manager module +- - This breaks imports of those binaries but means changes only require one PR + +### Pros + +- Each binary can be patched independently +- - Side effects of a patch are limited to one binary when only that binary has the dependency +- - This includes forking a dependency or needing to `replace` one +- - Gives us more control over our own destiny + +- Core go.mod dependencies are reduced +- - All importers of `github.com/cert-manager/cert-manager` have fewer transitive dependencies +- - Reduced chance of dependency conflicts for all importers +- - Including us - in subprojects! +- - Many people need to import cert-manager! (pkg/apis, etc). +- - We might split things more in the future - this is a good first step + +- Lays the groundwork for further splitting out binaries / packages +- - This is the start of what we'll do if we want cmctl to be its own repo +- - Or splitting `pkg/apis` into a separate module +- - Or splitting issuers into a module (to isolate cloud SDK dependencies) + +### Cons + +- Using local `replace` statements for binaries will break imports for those binaries +- - We assume this won't be too destructive in most cases +- - If we need to make binaries importable again, we can change them to use regular import statements +- - That would require two PRs in the event that we need to change the binary + the core module at the same time +- - If the binary would've ended up in a separate repo anyway (e.g. cmctl) we'd have done this eventually + +- Increased complexity in working with the codebase +- - E.g. `go test ./...` no longer tests _everything_, since it won't recurse into modules +- - This can be alleviated with some Makefile work - `make test` can still test everything + +## Longer Form Problem Statement **In short:** Some of our dependencies are complex which makes them hard to upgrade in already-released versions From 07cd2e37824d7a0b68b4be908fd104617469e148 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Wed, 24 May 2023 09:34:30 +0100 Subject: [PATCH 2/4] review suggestion Signed-off-by: Ashley Davis --- design/20230302.gomod.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/design/20230302.gomod.md b/design/20230302.gomod.md index 86afcebce..0fbdd1003 100644 --- a/design/20230302.gomod.md +++ b/design/20230302.gomod.md @@ -10,7 +10,7 @@ The intention here is to describe what we did and what we discovered, with an ey - It's hard or impossible to upgrade our dependencies months after a release - We won't change our conservative approach to backports -- Fewer dependencies in our core go module is a good thing +- Having fewer dependencies in our go modules is a good thing - It's OK if people can't import our binaries as go modules ### Solution From cc89050c0277c9902a102964b85209b934bf7312 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Fri, 26 May 2023 11:55:42 +0100 Subject: [PATCH 3/4] fix double indented dashes for github markdown Signed-off-by: Ashley Davis --- design/20230302.gomod.md | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/design/20230302.gomod.md b/design/20230302.gomod.md index 0fbdd1003..3bd94e571 100644 --- a/design/20230302.gomod.md +++ b/design/20230302.gomod.md @@ -18,39 +18,39 @@ The intention here is to describe what we did and what we discovered, with an ey - Create a go module for each binary - Create go modules for integration and e2e tests - Utilise local replace statements where possible -- - i.e. Binaries have a local replace for the core cert-manager module -- - This breaks imports of those binaries but means changes only require one PR + - i.e. Binaries have a local replace for the core cert-manager module + - This breaks imports of those binaries but means changes only require one PR ### Pros - Each binary can be patched independently -- - Side effects of a patch are limited to one binary when only that binary has the dependency -- - This includes forking a dependency or needing to `replace` one -- - Gives us more control over our own destiny + - Side effects of a patch are limited to one binary when only that binary has the dependency + - This includes forking a dependency or needing to `replace` one + - Gives us more control over our own destiny - Core go.mod dependencies are reduced -- - All importers of `github.com/cert-manager/cert-manager` have fewer transitive dependencies -- - Reduced chance of dependency conflicts for all importers -- - Including us - in subprojects! -- - Many people need to import cert-manager! (pkg/apis, etc). -- - We might split things more in the future - this is a good first step + - All importers of `github.com/cert-manager/cert-manager` have fewer transitive dependencies + - Reduced chance of dependency conflicts for all importers + - Including us - in subprojects! + - Many people need to import cert-manager! (pkg/apis, etc). + - We might split things more in the future - this is a good first step - Lays the groundwork for further splitting out binaries / packages -- - This is the start of what we'll do if we want cmctl to be its own repo -- - Or splitting `pkg/apis` into a separate module -- - Or splitting issuers into a module (to isolate cloud SDK dependencies) + - This is the start of what we'll do if we want cmctl to be its own repo + - Or splitting `pkg/apis` into a separate module + - Or splitting issuers into a module (to isolate cloud SDK dependencies) ### Cons - Using local `replace` statements for binaries will break imports for those binaries -- - We assume this won't be too destructive in most cases -- - If we need to make binaries importable again, we can change them to use regular import statements -- - That would require two PRs in the event that we need to change the binary + the core module at the same time -- - If the binary would've ended up in a separate repo anyway (e.g. cmctl) we'd have done this eventually + - We assume this won't be too destructive in most cases + - If we need to make binaries importable again, we can change them to use regular import statements + - That would require two PRs in the event that we need to change the binary + the core module at the same time + - If the binary would've ended up in a separate repo anyway (e.g. cmctl) we'd have done this eventually - Increased complexity in working with the codebase -- - E.g. `go test ./...` no longer tests _everything_, since it won't recurse into modules -- - This can be alleviated with some Makefile work - `make test` can still test everything + - E.g. `go test ./...` no longer tests _everything_, since it won't recurse into modules + - This can be alleviated with some Makefile work - `make test` can still test everything ## Longer Form Problem Statement From 5bfd3078cac08a10f454ddd2fe6a4995a26b8588 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Fri, 26 May 2023 12:00:54 +0100 Subject: [PATCH 4/4] add note about workspaces Signed-off-by: Ashley Davis --- design/20230302.gomod.md | 1 + 1 file changed, 1 insertion(+) diff --git a/design/20230302.gomod.md b/design/20230302.gomod.md index 3bd94e571..e0d3f79f5 100644 --- a/design/20230302.gomod.md +++ b/design/20230302.gomod.md @@ -51,6 +51,7 @@ The intention here is to describe what we did and what we discovered, with an ey - Increased complexity in working with the codebase - E.g. `go test ./...` no longer tests _everything_, since it won't recurse into modules - This can be alleviated with some Makefile work - `make test` can still test everything + - Go Workspaces (`go.work`) can also help in development environments to make things simpler ## Longer Form Problem Statement