|
| 1 | +--- |
| 2 | +name: dev |
| 3 | +description: > |
| 4 | + Ground rules for working in the Tika codebase — git policy, Maven |
| 5 | + wrapper/repo conventions, building and testing specific modules, code and |
| 6 | + test conventions, pre-commit checks. Load at session start for any Tika |
| 7 | + development task. |
| 8 | +--- |
| 9 | + |
| 10 | +<!-- |
| 11 | +Licensed to the Apache Software Foundation (ASF) under one or more |
| 12 | +contributor license agreements. See the NOTICE file distributed with |
| 13 | +this work for additional information regarding copyright ownership. |
| 14 | +The ASF licenses this file to You under the Apache License, Version 2.0 |
| 15 | +(the "License"); you may not use this file except in compliance with |
| 16 | +the License. You may obtain a copy of the License at |
| 17 | +
|
| 18 | + http://www.apache.org/licenses/LICENSE-2.0 |
| 19 | +
|
| 20 | +Unless required by applicable law or agreed to in writing, software |
| 21 | +distributed under the License is distributed on an "AS IS" BASIS, |
| 22 | +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 23 | +See the License for the specific language governing permissions and |
| 24 | +limitations under the License. |
| 25 | +--> |
| 26 | + |
| 27 | +# Tika Development Skill |
| 28 | + |
| 29 | +Guidelines and checklist for developing against the Apache Tika codebase. |
| 30 | + |
| 31 | +## Question the direction, not just the code |
| 32 | + |
| 33 | +Before optimizing a change — yours or a PR's — ask whether it should exist: |
| 34 | +does it belong in Tika, is the complexity proportional to the need, would |
| 35 | +config, an existing mechanism, a plugin, or documentation serve the use case |
| 36 | +more cheaply? Every merged feature is surface the project maintains for |
| 37 | +decades. Steelman the use case first and question the vehicle, not the goal; |
| 38 | +pushback must name a concrete cost or a simpler path — never taste alone, and |
| 39 | +never no for the sake of no. "The direction is right" is a valid conclusion. |
| 40 | + |
| 41 | +## Git Policy (default — personally overridable) |
| 42 | + |
| 43 | +Never run `git commit` or `git push` — no commits of any kind, including |
| 44 | +merge commits. If a merge is needed, use `git merge --no-commit --no-ff` |
| 45 | +and hand back. Stage files and provide the suggested commit message for |
| 46 | +the user to run. |
| 47 | + |
| 48 | +Never write to GitHub (PR comments, reviews, issues, labels, merges). |
| 49 | +Read-only `gh` is fine. |
| 50 | + |
| 51 | +**Precedence**: these are conservative defaults for *workflow* — actions on |
| 52 | +the contributor's own machine and accounts. A contributor's personal agent |
| 53 | +configuration (their own skills, CLAUDE.md/AGENTS.md, settings) may override |
| 54 | +them. Everything else in this file — code and comment conventions, test |
| 55 | +discipline, hygiene, pre-commit checks — governs what lands in the repo and |
| 56 | +is project policy: personal configuration does not override it. |
| 57 | + |
| 58 | +## Session Start Checklist |
| 59 | + |
| 60 | +1. **Local Maven repo** — Default to an in-repo `.local_m2_repo` |
| 61 | + (via `-Dmaven.repo.local=$(pwd)/.local_m2_repo`) unless the user says |
| 62 | + otherwise. This isolates builds from the shared `~/.m2/repository` and |
| 63 | + avoids polluting or being affected by other projects. |
| 64 | + |
| 65 | +2. **Maven wrapper** — Use `./mvnw`; fall back to a system Maven (3.9+) |
| 66 | + only if the wrapper is absent. |
| 67 | + |
| 68 | +3. **Merge conflicts** — Check `git status` for `UU` files and resolve |
| 69 | + before building. |
| 70 | + |
| 71 | +## Maven Rules |
| 72 | + |
| 73 | +- **Always include `clean`** in every `./mvnw` invocation. |
| 74 | + Stale classes in `target/` cause hard-to-debug failures. |
| 75 | + ```bash |
| 76 | + ./mvnw clean compile -pl <module> ... # not just: mvnw compile |
| 77 | + ./mvnw clean test -pl <module> ... # not just: mvnw test |
| 78 | + ./mvnw clean install -pl <module> ... # not just: mvnw install |
| 79 | + ``` |
| 80 | + |
| 81 | +- **Always use absolute path for local repo**: |
| 82 | + ```bash |
| 83 | + -Dmaven.repo.local=$(pwd)/.local_m2_repo |
| 84 | + ``` |
| 85 | + |
| 86 | +- **Fast builds with `-Pfast`** — use the `fast` profile to skip |
| 87 | + tests, checkstyle, spotless, and rat in one flag. Prefer this over |
| 88 | + individual `-D` skip flags when you want a quick build (e.g., |
| 89 | + installing for downstream consumers or eval runs): |
| 90 | + ```bash |
| 91 | + ./mvnw clean install -pl <module> -am -Pfast \ |
| 92 | + -Dmaven.repo.local=$(pwd)/.local_m2_repo |
| 93 | + ``` |
| 94 | + Run **without** `-Pfast` before final commit to catch formatting |
| 95 | + and style issues. License (rat) checks run only under `-Ppedantic` |
| 96 | + (or explicit `apache-rat:check`), not in default builds. |
| 97 | + |
| 98 | + **`-Pfast` skips test *execution*** (by design): a green `-Pfast` |
| 99 | + build — including `-Pfast test` — has run zero tests, and stale |
| 100 | + `target/surefire-reports/*` will look current. Verify with a plain |
| 101 | + (non-`-Pfast`) `test` run. |
| 102 | + |
| 103 | +- **Forked JVM tests** — Integration tests in `tika-pipes` fork new |
| 104 | + JVMs that load classes from the local Maven repo, not from |
| 105 | + `target/classes`. You must `./mvnw clean install -Pfast` the |
| 106 | + changed modules before running integration tests that fork. |
| 107 | + |
| 108 | +## Building Specific Modules |
| 109 | + |
| 110 | +```bash |
| 111 | +# Single module (with dependencies) |
| 112 | +./mvnw clean compile -pl <module> -am \ |
| 113 | + -Dmaven.repo.local=$(pwd)/.local_m2_repo |
| 114 | + |
| 115 | +# Run a single test class |
| 116 | +./mvnw clean test -pl <module> -Dtest=<TestClass> \ |
| 117 | + -Dmaven.repo.local=$(pwd)/.local_m2_repo -Dcheckstyle.skip=true |
| 118 | + |
| 119 | +# Install for downstream consumers (tika-app, integration tests) |
| 120 | +./mvnw clean install -pl <module> -am -Pfast \ |
| 121 | + -Dmaven.repo.local=$(pwd)/.local_m2_repo |
| 122 | +``` |
| 123 | + |
| 124 | +## Common Module Paths |
| 125 | + |
| 126 | +| Module | Path | |
| 127 | +|--------|------| |
| 128 | +| tika-core | `tika-core` | |
| 129 | +| tika-app | `tika-app` | |
| 130 | +| tika-server | `tika-server/tika-server-core` | |
| 131 | +| tika-eval | `tika-eval/tika-eval-app` | |
| 132 | +| Pipes core | `tika-pipes/tika-pipes-core` | |
| 133 | +| Pipes API | `tika-pipes/tika-pipes-api` | |
| 134 | +| Async CLI | `tika-pipes/tika-async-cli` | |
| 135 | + |
| 136 | +## Code Conventions |
| 137 | + |
| 138 | +- ASF License 2.0 header on all Java files |
| 139 | +- Spotless formatter runs during build — don't fight it |
| 140 | +- Tests use `@TempDir Path tmp` for temp directories |
| 141 | +- No emojis in code or comments |
| 142 | +- **Comments**: every comment must earn its place — one short line by |
| 143 | + default; multi-line only for a genuinely non-obvious WHY (subtle |
| 144 | + invariant, workaround, spec quirk). Never restate the code, narrate the |
| 145 | + next line, justify the change to a reviewer, or describe past states of |
| 146 | + the code. |
| 147 | +- **Input files are hostile**: bound anything derived from document content |
| 148 | + (loop counts, allocations, timeouts); release external processes, temp |
| 149 | + files, and pool slots on every failure path. |
| 150 | +- **No local/machine-specific paths** in committed code, tests, docs, or |
| 151 | + config — never `/home/<user>`, `/Users/<user>`, `C:\Users\<user>`, or a |
| 152 | + personal `~/data/...`. Use a placeholder (`<workdir>/`, `<corpus>`), |
| 153 | + `@TempDir`, or an in-repo `src/test/resources` fixture instead. *Only* |
| 154 | + legitimate exception: a path that is the data under test (e.g. an expected |
| 155 | + metadata value extracted from a test document) — leave those untouched. |
| 156 | + |
| 157 | +## Test Discipline |
| 158 | + |
| 159 | +- A behavioral change gets a regression test that fails without it. Where |
| 160 | + impractical (timing, native binaries, external services, kill paths), say |
| 161 | + so explicitly and name the next-best check. |
| 162 | +- Cover error paths and the configuration/mode matrix — a behavior verified |
| 163 | + in only one parse mode or config shape is a gap (RMETA-only tests miss |
| 164 | + CONCATENATE-only bugs). |
| 165 | +- Keep tests non-duplicative: don't add a test whose failure another test |
| 166 | + already guarantees. |
| 167 | +- Where there's bang for the buck, prefer parameterized tests over |
| 168 | + copy-pasted cases, randomized inputs over hand-picked ones (log the seed |
| 169 | + so failures reproduce), and fuzzing for parsers and format/boundary |
| 170 | + arithmetic. Don't force it on code a couple of fixed cases fully cover. |
| 171 | + |
| 172 | +## Metadata Keys & Schema Registry |
| 173 | + |
| 174 | +Adding/renaming a metadata key touches the committed, build-gated registry in |
| 175 | +`tika-metadata-schema` — regeneration has real traps. See |
| 176 | +`.skills/metadata-schema/SKILL.md`. |
| 177 | + |
| 178 | +## Testing an End-to-End Change |
| 179 | + |
| 180 | +When a change affects parsing output (e.g., new parser behavior, |
| 181 | +encoding fix), run a before/after comparison using tika-eval. |
| 182 | +See `.skills/tika-eval-compare/SKILL.md` for the full procedure. |
| 183 | + |
| 184 | +## Pre-Commit Checks |
| 185 | + |
| 186 | +```bash |
| 187 | +# Full compile with checkstyle (catches formatting issues) |
| 188 | +./mvnw clean compile -pl <module> -am \ |
| 189 | + -Dmaven.repo.local=$(pwd)/.local_m2_repo |
| 190 | + |
| 191 | +# Run module tests |
| 192 | +./mvnw clean test -pl <module> \ |
| 193 | + -Dmaven.repo.local=$(pwd)/.local_m2_repo |
| 194 | +``` |
| 195 | + |
| 196 | +Also before commit: |
| 197 | + |
| 198 | +- Commit message / PR title references a JIRA ticket (`TIKA-XXXX`). |
| 199 | +- CHANGES entry for user-visible changes. |
| 200 | +- New dependencies: ASF-compatible license; LICENSE/NOTICE updated. |
| 201 | + |
| 202 | +Scan the staged diff for machine-specific local paths before committing |
| 203 | +(see Code Conventions). Added lines only; review any hit by hand — a test |
| 204 | +fixture's expected value is allowed, a real config/doc/code path is not: |
| 205 | + |
| 206 | +```bash |
| 207 | +git diff --cached -U0 | grep -E '^\+' \ |
| 208 | + | grep -nE '/home/[A-Za-z0-9._-]+|/Users/[A-Za-z0-9._-]+|[A-Za-z]:\\+Users|~/data/' \ |
| 209 | + && echo "^ local path in staged diff — replace with a placeholder/fixture" |
| 210 | +``` |
0 commit comments