This is an automated email from the ASF dual-hosted git repository.
delei pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fesod.git
The following commit(s) were added to refs/heads/main by this push:
new a6dbe80e docs: add Copilot repository and code review instructions
(#958)
a6dbe80e is described below
commit a6dbe80e53e987023cb0f4191d277acb10ba91c7
Author: ian zhang <[email protected]>
AuthorDate: Sat Jul 25 13:54:44 2026 +0800
docs: add Copilot repository and code review instructions (#958)
* docs: add Copilot repository and code review instructions
- .github/copilot-instructions.md: repository-wide instructions covering
project overview, build/test commands, code style, module structure,
testing conventions, and CI checks
- .github/instructions/code-review.instructions.md: path-specific Java
review instructions with 8 priority checks and a pitfalls table derived
from real bug fixes (sql.Date toInstant, negative value validation,
premature ThreadLocal cleanup, missing defensive copy, FileWriter charset)
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
* docs: address Copilot review comments on PR #958
copilot-instructions.md:
- Fix misleading 'builds on JDK 17+' — clarify CI builds on JDK 8-25
- Remove unclear '(\#|\ pattern)' parenthetical, explain Spotless
importOrder
- Attribute license header enforcement to Hawkeye workflow, not Spotless
- Replace non-existent 'fesod-parent/' dir with '(root)' in module tree
- Fix '@DisplayName(ReplaceUnderscores)' to reference
junit-platform.properties
code-review.instructions.md:
- Attribute license header enforcement to Hawkeye workflow, not Spotless
- Fix backslash-escaped '\|\|' rendering issue in markdown table
- Use 'StandardCharsets.UTF_8' instead of 'UTF_8' for consistency
---------
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---
.github/copilot-instructions.md | 84 ++++++++++++++++++++++++
.github/instructions/code-review.instructions.md | 71 ++++++++++++++++++++
2 files changed, 155 insertions(+)
diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
new file mode 100644
index 00000000..7348fb74
--- /dev/null
+++ b/.github/copilot-instructions.md
@@ -0,0 +1,84 @@
+# Apache Fesod (Incubating) — Copilot Repository Instructions
+
+## Project Overview
+
+Apache Fesod (Incubating) is a Java library for processing spreadsheets
(XLS/XLSX/CSV) without OOM, derived from Alibaba EasyExcel. It uses Apache POI
under the hood with streaming SAX-based reading. The project is an Apache
incubator project and must follow ASF conventions.
+
+- **Language**: Java 8+ (source/target = 1.8; CI builds on JDK 8, 11, 17, 21,
25 — JDK 17+ is recommended for local development)
+- **Build**: Maven 3.6.3 via `./mvnw` wrapper (project docs require 3.9+, but
wrapper currently pins 3.6.3; both work)
+- **CI matrix**: JDK 8, 11, 17, 21, 25
+
+## Build & Test Commands
+
+```bash
+# Build (skips tests by default)
+./mvnw clean install -DskipTests
+
+# Run tests (must explicitly enable)
+./mvnw clean package -Dmaven.test.skip=false -pl
fesod-common,fesod-shaded,fesod-sheet,fesod-examples/fesod-sheet-examples
+
+# Format check / auto-format
+./mvnw spotless:check
+./mvnw spotless:apply
+
+# Run tests by tag
+./mvnw test -Dtest.groups=unit # fast unit tests only
+./mvnw test -Dtest.excludedGroups=fuzz # exclude slow fuzz tests
+
+# Full install with javadoc (CI validation)
+./mvnw install -B -V
+./mvnw javadoc:javadoc
+```
+
+## Code Style & Conventions
+
+- **Formatter**: Palantir Java Format (enforced by Spotless, no checkstyle)
+- **Imports**: Static imports first, then regular imports; no unused imports
(enforced by Spotless `importOrder` with `\#|` separator)
+- **Indent**: 4 spaces
+- **License header**: ASF Apache 2.0 header required on all `.java`, `.xml`,
`.yml`, `.toml` files. Header template at `tools/spotless/license-header.txt`;
enforced by the Hawkeye workflow (`.github/workflows/license-check.yml`), not
Spotless
+- **Lombok**: Allowed; config in `lombok.config` (`toString.callSuper = CALL`,
`equalsAndHashCode.callSuper = CALL`)
+- **Commits**: English, format `type: description` (types: `fix`, `feat`,
`refactor`, `test`, `docs`, `chore`, `style`, `dependency`)
+
+## Module Structure
+
+```
+(root) # Root POM (artifactId: fesod-parent), dependency &
plugin management
+├── fesod-common/ # Zero-dependency utilities
(org.apache.fesod.common.util)
+├── fesod-shaded/ # Relocated Spring ASM/cglib (org.apache.fesod.shaded)
+├── fesod-bom/ # BOM for downstream consumers
+├── fesod-sheet/ # Core library: read/write XLS/XLSX/CSV via POI
+│ ├── src/main/java/org/apache/fesod/sheet/
+│ │ ├── FesodSheet.java # Main entry: FesodSheet.read() /
FesodSheet.write()
+│ │ ├── analysis/ # Read pipeline (v03=XLS BIFF, v07=XLSX SAX,
csv)
+│ │ ├── write/ # Write pipeline (builder, executor, handler
chains)
+│ │ ├── metadata/ # Data models, builders, csv/ property/
+│ │ ├── converters/ # Type conversion framework (by Java type)
+│ │ └── util/ # DateUtils, NumberUtils, WorkBookUtil, etc.
+│ └── src/test/java/org/apache/fesod/sheet/
+│ └── testkit/ # Test infrastructure (NOT a separate module)
+│ ├── Tags.java # @Tag constants: unit, round-trip, read,
write, format, fuzz
+│ ├── base/ # AbstractExcelTest (round-trip base)
+│ ├── assertions/ # ExcelAssertions fluent API
+│ └── builders/ # TestDataBuilder
+└── fesod-examples/ # Usage examples
+```
+
+## Testing Conventions
+
+- **JUnit 5** with `PER_CLASS` lifecycle and `ReplaceUnderscores` display name
generator (configured globally in
`fesod-sheet/src/test/resources/junit-platform.properties`, not via per-class
annotations)
+- **Tags** (use `@Tag(Tags.XXX)`):
+ - `unit` — pure logic, no file I/O
+ - `round-trip` — write-then-read via `AbstractExcelTest`
+ - `read` / `write` — single-direction tests
+ - `format` — CSV/charset/date format tests
+ - `fuzz` — Jazzer property-based (slow, excluded in PR CI)
+- **Real-file tests**: Prefer `AbstractExcelTest.createTempFile()` +
`ExcelAssertions` for integration tests
+- **Test data**: Use `TestDataBuilder.simpleData(n)` or model classes in
`testkit/models/`
+
+## CI Checks (must pass before merge)
+
+1. `spotless:check` — format
+2. License header (hawkeye)
+3. Multi-JDK test matrix (8/11/17/21/25)
+4. `mvn install` + `mvn javadoc:javadoc`
+5. CodeQL security scan
diff --git a/.github/instructions/code-review.instructions.md
b/.github/instructions/code-review.instructions.md
new file mode 100644
index 00000000..52a9bf10
--- /dev/null
+++ b/.github/instructions/code-review.instructions.md
@@ -0,0 +1,71 @@
+---
+applyTo: "**/*.java"
+---
+
+# Code Review Instructions for Apache Fesod
+
+## Priority Checks
+
+### 1. Java 8 Source Compatibility
+
+The project targets Java 8 (`source/target = 1.8`) but CI runs on JDK 8
through 25. Review for:
+
+- **No Java 9+ APIs**: Methods like `List.of()`, `Map.of()`, `String.strip()`,
`InputStream.readAllBytes()` do not exist in Java 8. Use
`Collections.singletonList()`, `new HashMap<>()`, `String.trim()`, or Apache
Commons equivalents instead.
+- **`java.sql.Date.toInstant()` / `java.sql.Time.toInstant()`**: These throw
`UnsupportedOperationException` on Java 9+ because the types are
date-only/time-only. When converting `java.util.Date` subclasses, always use
`instanceof` checks and call `toLocalDate()` / `toLocalTime()` instead. This
applies to both `WriteCellData` and `CsvCell` code paths — keep them consistent.
+- **No `var` keyword**: Java 8 does not support `var`. Always declare explicit
types.
+
+### 2. Immutability & Defensive Copies
+
+Builder methods that accept or return mutable collections must protect
internal state:
+
+- **`head(Consumer)` pattern**: When
`AbstractParameterBuilder.head(Consumer<HeadBuilder>)` stores the result of
`DefaultHeadBuilder.define()`, it must wrap it with
`toMutableListIfNecessary()` to create a defensive copy. Otherwise,
`ExcelHeadProperty.initHeadRowNumber()` will mutate the builder's internal list
during write, corrupting subsequent reuses.
+- **General rule**: Any builder method that stores a collection from an
external source must copy it. Any getter that returns an internal collection
should document whether it's modifiable.
+
+### 3. Input Validation & Boundary Values
+
+- **Negative values**: Validation logic must handle all invalid values, not
just sentinel defaults. For example, `@FreezePane(leftmostColumn = -1)` uses
`-1` as "use default", but `leftmostColumn = -5` must also fall back to default
— check `value == null || value < 0`, not just `value == -1`.
+- **Null safety**: Public API entry points must validate null inputs and fail
fast with a clear `IllegalArgumentException` rather than NPE deep in the call
stack.
+
+### 4. Resource & Lifecycle Management
+
+- **ThreadLocal cleanup**: When setting thread-local state (e.g.,
`Biff8EncryptionKey.setCurrentUserPassword()`), do not clear it prematurely in
a `finally` block if the state is needed by a later phase. The cleanup must
happen only after all consumers have finished — typically in a dedicated
`clearXxx()` method called at the end of the overall operation.
+- **Try-with-resources**: Always use try-with-resources for `ExcelWriter`,
`ExcelReader`, `Workbook`, `OutputStream`, `Writer`. Never leave workbook
resources unclosed.
+- **File handles**: After writing Excel files via POI, ensure the workbook is
fully closed before the file handle is released.
+
+### 5. API Consistency Across Code Paths
+
+The same operation may be implemented in multiple places. When fixing a bug in
one path, check all parallel paths:
+
+- `WriteCellData` vs `CsvCell`: Both handle date conversion. A fix in one must
be mirrored in the other.
+- `FesodSheet` vs `FastExcel` vs `EasyExcel`: These are aliases. A behavior
change in the modern API must not break legacy aliases.
+- `ExcelWriterBuilder` vs `ExcelReaderBuilder`: Symmetric builder patterns
should stay symmetric.
+
+### 6. Test Quality
+
+- **Encoding consistency**: When tests write files, always specify the charset
explicitly. Use `Files.newBufferedWriter(path, StandardCharsets.UTF_8)` instead
of `new FileWriter(file)` (platform-default charset). The test must match the
encoding configured in the production code (e.g., `CsvWorkbook` uses UTF-8).
+- **Deprecated APIs in tests**: Avoid `new java.util.Date(year-1900, month,
day)` — use `Calendar` or `java.time` types converted via `Date.from()`.
Deprecated constructors generate warnings that pollute build output.
+- **Tag selection**: Use the correct `@Tag`:
+ - `@Tag(Tags.UNIT)` for pure logic (no file I/O)
+ - `@Tag(Tags.ROUND_TRIP)` for write-then-read integration tests
+ - `@Tag(Tags.FORMAT)` for CSV/charset/date format tests
+ - `@Tag(Tags.WRITE)` for write-only tests
+- **Real-file verification**: For bug fixes that affect file output, add at
least one test that writes a real file and reads it back (via `ExcelAssertions`
or POI `WorkbookFactory`). Code-level assertions alone may miss integration
issues.
+- **Regression evidence**: When fixing a bug, verify the test actually fails
without the fix (revert source, run test, confirm failure, restore fix).
+
+### 7. ASF & Project Conventions
+
+- **License header**: Every new `.java` file must have the ASF Apache 2.0
header (see `tools/spotless/license-header.txt`). License headers are enforced
by the Hawkeye workflow (`.github/workflows/license-check.yml`), not Spotless.
+- **Package naming**: All code must be under `org.apache.fesod.*`. Never use
`com.alibaba.*` or `cn.idev.*` (legacy packages from EasyExcel era).
+- **Lombok**: `toString.callSuper = CALL` and `equalsAndHashCode.callSuper =
CALL` are enforced via `lombok.config`. Subclasses must call super.
+- **No checked exceptions in public API**: Wrap checked exceptions in
`ExcelGenerateException` or `ExcelAnalysisException` rather than declaring
`throws` on builder methods.
+
+### 8. Common Pitfalls (from real bug fixes)
+
+| Pattern | Problem | Fix |
+|---------|---------|-----|
+| `value.toInstant()` on `java.sql.Date`/`Time` |
`UnsupportedOperationException` on Java 9+ | Use `instanceof` +
`toLocalDate()`/`toLocalTime()` |
+| `value == -1` validation | Misses other negative values | Check `value ==
null` or `value < 0` |
+| `try { setThreadLocal(x); } finally { setThreadLocal(null); }` | Clears
state needed by later phase | Remove premature cleanup; clean up at end of full
operation |
+| `parameter().setHead(DefaultHeadBuilder.define(c))` | Stores internal list
reference | Wrap with `toMutableListIfNecessary()` |
+| `new FileWriter(file)` in tests | Platform-default charset | Use
`Files.newBufferedWriter(path, StandardCharsets.UTF_8)` |
+| `new FileOutputStream(file)` outside try-with-resources | Resource leak on
exception | Wrap in `try (FileOutputStream fos = new FileOutputStream(file)) {
... }` |
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
