Skip to content

update typing skill - #35609

Open
anna-shakhova wants to merge 2 commits into
DevExpress:mainfrom
anna-shakhova:update_typing_skill_main
Open

anna-shakhova wants to merge 2 commits into
DevExpress:mainfrom
anna-shakhova:update_typing_skill_main

Conversation

@anna-shakhova

Copy link
Copy Markdown
Contributor

No description provided.

@anna-shakhova anna-shakhova self-assigned this Oct 9, 2026
Copilot AI balanced review requested due to automatic review settings October 9, 2026 14:18

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.

🟡 Changes recommended

Two instructions inaccurately describe iteration and transpilation behavior and could cause incorrect typing work.

2 open findings
What changed in this PR

Updates the typing skill with expanded workflow guidance, behavior-preservation traps, grid conventions, and TypeScript fixes.

Changes:

  • Adds typing workflow and verification guidance.
  • Documents additional refactoring traps and grid-specific patterns.
  • Expands recommended fixes for TypeScript and ESLint issues.
File Description
SKILL.md Updates workflow and verification commands.
traps.md Adds behavior-change warnings.
grids.md Adds grid typing conventions.
fixes.md Expands typing fixes and workarounds.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .claude/skills/typing-m-files/SKILL.md Outdated
Comment thread .claude/skills/typing-m-files/traps.md Outdated
Copilot AI balanced review requested due to automatic review settings October 9, 2026 14:35

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.

🔵 Needs a closer look

Several new instructions reference obsolete paths or recommend an unsafe postfix-increment replacement.

0 open findings

2 resolved since last review
Previously missed (4)

In code that hasn't changed since last review

Low severity Update coordination check to the current shared types path

.claude/​skills/​typing-m-files/​SKILL.md:53

This coordination check targets a file that no longer exists. The shared grid hub is packages/devextreme/js/__internal/grids/grid_core/types.ts, so filtering for grid_core/m_types.ts silently returns no matching PRs and defeats the new conflict-avoidance step. Update both the example and the query to the current path.

Low severity Limit string increment replacement to standalone expressions

.claude/​skills/​typing-m-files/​fixes.md:141

The string-operand recommendation is unsafe when the value of postfix ++ is consumed: postfix returns the old numeric value, but x = Number(x) + 1 returns the incremented value. Restrict this replacement to standalone increments and document that expression uses need to preserve the old numeric result.

Low severity Correct obsolete paths in the new type placement rule

.claude/​skills/​typing-m-files/​grids.md:13

This newly added rule names two paths that are no longer present: the shared type hub is grid_core/types.ts, and the modules implementation is now under grid_core/modules/modules.ts. As written, it directs agents to add types to a nonexistent m_types.ts file.

Low severity Preserve postfix increment result when its value is consumed

.claude/​skills/​typing-m-files/​traps.md:8

x = Number(x) + 1 is equivalent only when the increment's expression result is discarded. Postfix x++ evaluates to the old numeric value, while the assignment evaluates to the new value, so replacing a consumed increment (for example in an argument or template) changes behavior. Please qualify this guidance and require preserving the old numeric value when it is consumed.

🧠 Review effort: Balanced

@anna-shakhova
anna-shakhova force-pushed the update_typing_skill_main branch from 76cc50c to 1b16581 Compare October 9, 2026 14:44
Copilot AI balanced review requested due to automatic review settings October 9, 2026 14:44

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.

🟢 Approval recommended

The documentation updates are internally consistent and align with the current repository structure and tooling.

0 open findings

🧠 Review effort: Balanced

@anna-shakhova
anna-shakhova added this pull request to the merge queue Oct 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 9, 2026

This branch has not been deployed

No deployments
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