Skip to content

docs: add Copilot repository and code review instructions#958

Open
alaahong wants to merge 4 commits into
apache:mainfrom
alaahong:copilot-instructions
Open

docs: add Copilot repository and code review instructions#958
alaahong wants to merge 4 commits into
apache:mainfrom
alaahong:copilot-instructions

Conversation

@alaahong

Copy link
Copy Markdown
Member

Purpose of the pull request

Add GitHub Copilot custom instructions to improve automated code review quality and reduce repetitive review feedback on PRs. This follows the official GitHub Copilot repository instructions feature.

What's changed?

Background

Copilot code review on recent PRs (#954 etc.) raised valid points (e.g., FileWriter using platform-default charset, deprecated java.util.Date constructors) that could have been caught earlier with proper repository context. These instructions encode project-specific conventions so Copilot can provide more accurate and actionable review feedback.

Files added

  1. .github/copilot-instructions.md — Repository-wide instructions (applies to all Copilot interactions):

    • Project overview: Apache Fesod (Incubating), Java 8+, POI-based streaming Excel/CSV library
    • Build & test commands: ./mvnw wrapper, module-specific test commands, tag-based filtering
    • Code style: Palantir Java Format (Spotless), ASF license header, Lombok config
    • Module structure: fesod-common → fesod-shaded → fesod-bom → fesod-sheet dependency graph
    • Testing conventions: JUnit 5 @Tag system (unit, round-trip, read, write, format, fuzz)
    • CI checks: multi-JDK matrix (8/11/17/21/25), Spotless, license, CodeQL
  2. .github/instructions/code-review.instructions.md — Path-specific Java review instructions (applyTo: "**/*.java"):

    • 8 priority check dimensions derived from real bug fixes and Copilot review feedback
    • Common pitfalls table with 5 patterns extracted from actual fixes:
      • toInstant() on java.sql.Date/TimeUnsupportedOperationException
      • value == -1 validation missing other negative values
      • Premature ThreadLocal cleanup in finally blocks
      • Missing toMutableListIfNecessary() defensive copy in builders
      • FileWriter platform-default charset in tests
    • Each rule references the specific code path it applies to
image

Design decisions

  • Two files instead of one: Repository-wide instructions provide general context; path-specific instructions focus Copilot review on Java-specific concerns. This matches GitHub's recommended structure.
  • Pitfalls table format: Compact reference table is easier for Copilot to pattern-match against code than prose paragraphs.
  • No task-specific instructions: Per GitHub's guidance, instructions are general project conventions, not tied to specific issues or features.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

(No unit tests required — this PR adds documentation/configuration files only. Spotless pre-push hook passed: all 482 Java files clean.)

- .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)
@alaahong
alaahong requested a review from Copilot July 19, 2026 00:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds GitHub Copilot repository-wide and Java-specific code review instructions to encode Apache Fesod project context and recurring review pitfalls, improving automated review quality and reducing repetitive feedback.

Changes:

  • Adds repository-wide Copilot instructions covering project overview, build/test commands, module layout, testing conventions, and CI checks.
  • Adds Java path-specific code review instructions (via applyTo: "**/*.java") with prioritized review dimensions and a “common pitfalls” reference table.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
.github/copilot-instructions.md Adds repository-wide Copilot guidance (project context, build/test commands, conventions, module/test/CI overview).
.github/instructions/code-review.instructions.md Adds Java-only code review focus areas and a compact pitfalls table for common regressions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/copilot-instructions.md Outdated
Comment thread .github/copilot-instructions.md Outdated
Comment thread .github/instructions/code-review.instructions.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Comment thread .github/instructions/code-review.instructions.md Outdated
Comment thread .github/instructions/code-review.instructions.md Outdated
Comment thread .github/copilot-instructions.md Outdated
Comment thread .github/copilot-instructions.md Outdated
Comment thread .github/copilot-instructions.md Outdated
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants