feature-workflow

<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with this work for additional information regarding copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> Local override: `$TIKA_SKILLS_LOCAL/feature-workflow/LOCAL.md` (default `~/.tika-skills`), read after this file, wins on conflict. # Feature Workflow: spike, contract, cut, ship A complex feature's shape is learned by building it. Learning it *on the PR* costs a review round per lesson and reshapes what the next round reviews. Keep learning and shipping on different branches. ## 1. Spike Throwaway branch. Build end to end, roughly: no CHANGES, docs, or polish; change neighbors freely. Review it (`.skills/devs/pr-review/SKILL.md`) and let findings reshape interfaces. **Exit:** the last review changed edge-case handling, not an interface. While reviews still rename, split, or add methods, keep spiking (pr-review verdict "still spiking"). ## 2. Contract Write down what the spike taught, half a page per lifecycle object (open/close, acquire/release, publish/abort, spill, rewind): states and transitions, each method's behavior per state, resource ownership on success and every failure path, threading. Put it in the type's javadoc. Encode it as an `Abstract<Type>ContractTest` every implementation extends: close twice, abort then close, write after close, throw mid-write then close, resources released on both paths. This is what makes review converge. ## 3. Cut PRs Split *after* the spike, one contract (or tightly coupled group) per PR. Splitting before is guesswork and leaves one PR holding five contracts. Each PR carries only that contract's files, its contract test, CHANGES, docs. Everything else waits for its own PR or the todo doc. ## 4. Ship Per PR: one high-effort review, fix, one confirm pass on the delta. A third round means either a fix changed an interface (pull that piece back to the spike) or findings are out of scope (todo doc, not the fix commit). Never widen a PR during review. | You see | Do | |---|---| | Review adds/renames/splits a type or method | keep spiking; no PR yet | | Review finds edge cases only | write contract + test, cut PR | | PR review changes an interface | pull piece back to spike | | PR review finds out-of-scope issue | todo doc, not this PR | | Third review round on a PR | one of the two above applies | ## Cross-cutting refactors (chokepoints, wire formats) A refactor claiming "every X goes through Y" — a policy chokepoint, a single serialization route — is a contract too, with three extra rules. (Learned on TIKA-4848, where all three were violated and each violation became a post-merge review round.) - **Enforcement ships with the invariant, default-on.** The PR that creates the chokepoint also adds the build-breaking check (forbidden-apis ban, architecture test) that keeps call sites on it — every module, explicit opt-outs. Opt-in checks document an invariant; only default-on maintains it. Without this, each missed call site is a review finding instead of a compile failure. - **One thin end-to-end test per transport surface.** A knob that crosses a JVM boundary is exercised through each surface it rides (in-JVM, server, pipes fork, grpc) in the invariant's own PR. Diff review cannot see a parent-resolves/fork-deserializes seam; a wire test can. - **Producer and consumer change together.** Both halves of a serializer/deserializer or parent/fork protocol pair go in the same PR. Cutting by module puts every seam between two reviews, and seams are where cross-cutting bugs live. No compat shim without a caller: grep before deprecating-in-place. A shim for an API nothing calls is dead weight that still carries bugs.