diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 021fb6c92..4507980a7 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -4,6 +4,7 @@ - 2️⃣ PR body: Replace 'body...' with detailed description of the change. - 3️⃣ category: pick one, delete the rest. - 4️⃣ ticket: Replace #000 with link to a GitHub issue (or 'none' if PR is trivial). + - 5️⃣ feature_set: pick one (or delete completely if not applicable). 🧑‍🎓 Please review our contribution guide https://github.com/ObolNetwork/charon/blob/main/docs/contributing.md - 📜 Sign the Contributor License Agreement (CLA) when prompted. - 🌱 Starting with an issue, outlining the problem and proposed solution, is highly encouraged. @@ -15,3 +16,4 @@ body... category: bug feature refactor docs test fixbuild misc ticket: #000 +feature_set: alpha beta stable diff --git a/docs/branching.md b/docs/branching.md index 6273e9ef0..5d6d7768d 100644 --- a/docs/branching.md +++ b/docs/branching.md @@ -21,7 +21,7 @@ We follow [Trunk Based Development](https://trunkbaseddevelopment.com/) as a bra - Since a feature cannot be added as a single big merge of a big feature branch, tools and patterns are required that allow gradual controlled introduction of increment changes without breaking. - New code can be added as “dead code”. So not integrated into the actual program yet. Once it is properly complete, it can be integrated in a single PR. -- Some features should however not be enabled straight into prod/mainnet, but should be tested in dev/devnet or staging/testnet first. This can be achieved by simple [feature switches](https://trunkbaseddevelopment.com/feature-flags/) (if statements) that integrate new features based on the environment (e.g. only enable feature X in testnet). TODO(corver): Add feature switch functionality. +- Some features should however not be enabled straight into prod/mainnet, but should be rolled-out slowly being first tested in `alpha` (internal devnet only), then `beta` (internal and external testnet), and then only `stable` (enabled everywhere). This can be achieved by simple [feature switches](https://trunkbaseddevelopment.com/feature-flags/) (if statements) that enable features based on their `feature_set` status. - Another powerful pattern to gradually introduce change is [branching by abstraction](https://trunkbaseddevelopment.com/branch-by-abstraction/). This basically introduces an abstraction layer at the point where a new feature has to replace an old feature (like an interface). Using dependency injection, the new feature can be integrated during testing/staging while the old feature is still being used in production. - Note that both feature switches and/or abstraction layers used to roll out a feature should be removed once released to prod/main-net. diff --git a/docs/contributing.md b/docs/contributing.md index 2937e8457..d9b67dca7 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -85,7 +85,7 @@ Note: PRs can only be merged by obol-bulldozer bot. It is author's responsibilit - Ends with a list of tags (some required, others optional) (`^tag: value of this tag\n`): - `category`: required; one of: `refactor`, `bug`, `feature`, `docs`, `release`, `tidy`, `fixbuild`. - `ticket`: required; URL of the Github issue just a reference, E.g. `#123` or `none`. - - `phase`: optional; identifies the highest release cycle phase targeted by the change; `aplha`, `beta`, `gold` + - `feature_set`: optional; identifies the highest rollout status targeted by the change; `alpha`, `beta`, `stable` - Example: ``` runner/tracer: add jaeger otel exporter @@ -94,7 +94,7 @@ Adds the jaeger exporter to our opentelemetery infra. category: feature ticket: #206 -phase: gold +feature_set: stable ``` ### Dev tools, git hooks and linters. diff --git a/testutil/verifypr/verifypr.go b/testutil/verifypr/verifypr.go index 1c01dfab8..127f43326 100644 --- a/testutil/verifypr/verifypr.go +++ b/testutil/verifypr/verifypr.go @@ -133,6 +133,7 @@ func verifyBody(body string) error { prevLineEmpty bool foundCategory bool foundTicket bool + foundFeature bool ) for i, line := range strings.Split(body, "\n") { if i == 0 && strings.TrimSpace(line) == "" { @@ -204,6 +205,31 @@ func verifyBody(body string) error { foundTicket = true } + const featureTag = "feature_set:" + if strings.HasPrefix(line, featureTag) { + if foundFeature { + return errors.New("multiple ticket tag lines") + } + + set := strings.TrimSpace(strings.TrimPrefix(line, featureTag)) + + if set == "" { + return errors.New("feature_set tag empty") + } + + sets := map[string]bool{ + "alpha": true, + "beta": true, + "stable": true, + } + + if !sets[set] { + return errors.New("invalid feature_set tag not one of: alpha, beta, stable") + } + + foundFeature = true + } + prevLineEmpty = strings.TrimSpace(line) == "" } diff --git a/testutil/verifypr/verifypr_internal_test.go b/testutil/verifypr/verifypr_internal_test.go index de35ea7ca..757ee0f59 100644 --- a/testutil/verifypr/verifypr_internal_test.go +++ b/testutil/verifypr/verifypr_internal_test.go @@ -141,6 +141,22 @@ func TestBody(t *testing.T) { Body: "