Skip to content

Extract Kotlin guidelines to docs/KOTLIN.md and link across docs, skills, and agent files#2973

Merged
afranken merged 2 commits intomainfrom
copilot/extract-kotlin-guidelines
Feb 25, 2026
Merged

Extract Kotlin guidelines to docs/KOTLIN.md and link across docs, skills, and agent files#2973
afranken merged 2 commits intomainfrom
copilot/extract-kotlin-guidelines

Conversation

Copy link
Contributor

Copilot AI commented Feb 25, 2026

Kotlin-specific idioms, naming conventions, anti-patterns, and KDoc guidelines were duplicated across AGENTS.md, refactor/SKILL.md, and review/SKILL.md. This centralizes them into a single authoritative reference.

Changes

  • New docs/KOTLIN.md — canonical Kotlin reference covering: idioms (null safety, expression bodies, when, scope functions, string templates), common anti-patterns table, naming conventions, test naming (internal, backtick names), and KDoc guidelines
  • AGENTS.md — Code Style section condensed to a link; rules in DO/DON'T remain as-is
  • .claude/skills/refactor/SKILL.md — "Idiomatic Kotlin", "Common Anti-Patterns", "Meaningful Naming", and "KDoc" sections replaced with references to docs/KOTLIN.md
  • .claude/skills/review/SKILL.md — "Kotlin Idioms" section replaced with a reference
  • .claude/skills/test/SKILL.mddocs/KOTLIN.md added to the pre-reading list
  • .claude/skills/document/SKILL.mddocs/KOTLIN.md added to the documentation files table
  • docs/TESTING.md — Conventions section links to docs/KOTLIN.md for naming patterns
Original prompt

Extract generic, kotlin specific guidelines to docs/KOTLIN.md and link across documentation, skill and agent files.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…, skills, and agent files

Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
Copilot AI changed the title [WIP] Extract Kotlin specific guidelines to docs/KOTLIN.md Extract Kotlin guidelines to docs/KOTLIN.md and link across docs, skills, and agent files Feb 25, 2026
@afranken afranken requested review from afranken and Copilot February 25, 2026 20:02
@afranken afranken marked this pull request as ready for review February 25, 2026 20:05
Copy link
Contributor

Copilot AI left a comment

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 centralizes Kotlin-specific style and idiom guidance into a single canonical document (docs/KOTLIN.md) and updates contributor/agent documentation to link to it, reducing duplication and drift across the repo.

Changes:

  • Added docs/KOTLIN.md as the authoritative Kotlin guidelines reference (idioms, anti-patterns, naming, test naming, KDoc).
  • Replaced duplicated Kotlin guidance in agent/skill docs with links to docs/KOTLIN.md.
  • Updated testing documentation to reference docs/KOTLIN.md for Kotlin test naming conventions.

Reviewed changes

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

Show a summary per file
File Description
docs/TESTING.md Links Kotlin naming/test conventions to the new canonical Kotlin guidelines doc.
docs/KOTLIN.md Introduces the centralized Kotlin guidelines (idioms, anti-patterns, naming, test naming, KDoc).
AGENTS.md Condenses Kotlin guidance in Code Style to a single link to docs/KOTLIN.md.
.claude/skills/test/SKILL.md Adds docs/KOTLIN.md to pre-reading for writing tests.
.claude/skills/review/SKILL.md Replaces the Kotlin idioms checklist with a link to docs/KOTLIN.md.
.claude/skills/refactor/SKILL.md Replaces Kotlin idioms/anti-patterns/naming/KDoc details with references to docs/KOTLIN.md.
.claude/skills/document/SKILL.md Adds docs/KOTLIN.md to the documentation files table.

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


### Lambda Parameters
- Always name `it` in nested or non-trivial lambdas to avoid shadowing
- `.map { part -> ... }` instead of `.map { it.name }`
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

The lambda-parameter example doesn’t show an equivalent transformation: .map { it.name } becomes .map { part -> ... }, which changes semantics and may confuse readers. Consider using an equivalent explicit-parameter example (e.g., naming the parameter and still returning the same expression) or provide a nested-lambda example that actually demonstrates it shadowing.

Suggested change
- `.map { part -> ... }` instead of `.map { it.name }`
- `.map { part -> part.name }` instead of `.map { it.name }`

Copilot uses AI. Check for mistakes.
| `Collections.emptyList()` | `emptyList()` |
| `object.equals(other)` | `object == other` |
| `!(x is Foo)` / `!(list.contains(x))` | `x !is Foo` / `x !in list` |
| `for + add` loops | `.map { ... }` |
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

The anti-pattern entry “for + add loops → .map { ... }” is a bit too broad: map only fits 1:1 transforms, while many for+add loops are filtering/flattening/side-effecting (better covered by filter/mapNotNull/flatMap/forEach). Consider tightening the wording (e.g., “for loops that build a new list via add(f(x))”) or expanding the suggested alternatives to avoid misleading guidance in this canonical doc.

Suggested change
| `for + add` loops | `.map { ... }` |
| `for` loops that build a new list via `result.add(f(x))` | Collection ops: `.map` / `.filter` / `.mapNotNull` / `.flatMap` / `.forEach` as appropriate |

Copilot uses AI. Check for mistakes.
Copy link
Member

@afranken afranken left a comment

Choose a reason for hiding this comment

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

👍

@afranken afranken merged commit f75a30a into main Feb 25, 2026
11 checks passed
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.

3 participants