fanhuadesenlinnn/tdesktop
1
0
Fork 0
mirror of https://hubproxy.babadafafafafa.cn/https://github.com/telegramdesktop/tdesktop synced 2026-09-20 08:03:45 +08:00
Code Issues Packages Projects Releases Wiki Activity

[ai] Extract REVIEW.md for review task step.

Browse Source
This commit is contained in:
John Preston
2026-02-06 14:39:48 +04:00
parent 9468dad455
commit b0bcb2132c
4 changed files with 106 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.claude/commands/task.md
View File

@@ -334,12 +334,13 @@ You are a code review agent for Telegram Desktop (C++ / Qt).
Read these files:
- .ai/<feature-name>/context.md - Codebase context
- .ai/<feature-name>/plan.md - Implementation plan
- REVIEW.md - Style and formatting rules to enforce
<if R > 1, also read:>
- .ai/<feature-name>/review<R-1>.md - Previous review (to see what was already addressed)
Then run this command to see all changes made by the implementation:
git diff HEAD~<number-of-implementation-commits> -- . ":(exclude).ai"
(Ask git log to figure out how many commits back the implementation started, or diff against the base branch. The goal is to see ONLY the implementation diff, excluding .ai/ files.)
git diff HEAD~<number-of-implementation-commits>
(Ask git log to figure out how many commits back the implementation started, or diff against the base branch. The goal is to see the implementation diff.)
Then read the modified source files in full to understand changes in context.
@@ -359,7 +360,7 @@ REVIEW CRITERIA (in order of importance):
6. **Module structure**: Only in exceptional cases — if a large amount of newly added code (hundreds of lines) is logically distinct from the rest of its host module, suggest extracting it into a new module. But do NOT suggest new modules lightly: every module adds significant build overhead due to PCH and heavy template usage. Only suggest this when the new code is both large enough AND logically separated enough to justify it. At the same time, don't let modules grow into multi-thousand-line monoliths either.
7. **Style compliance**: Verify adherence to CLAUDE.md conventions — no unnecessary comments, `auto` usage, empty line before closing brace, operators at start of continuation lines, no hardcoded sizes (must use .style definitions), etc.
7. **Style compliance**: Verify adherence to REVIEW.md rules (empty line before closing brace, operators at start of continuation lines, minimize type checks with direct cast instead of is+as, no if-with-initializer when simpler alternatives exist) and CLAUDE.md conventions (no unnecessary comments, `auto` usage, no hardcoded sizes — must use .style definitions), etc.
IMPORTANT GUIDELINES:
- Review ONLY the changes made, not pre-existing code in the repository.

7
.codex/skills/task-think/PROMPTS.md
View File

@@ -86,14 +86,17 @@ If blocked by locked files or access errors, stop and report exact blocker.
```text
You are the review phase for task "<TASK>" in repository <REPO_ROOT>.
Read CLAUDE.md for the basic coding rules and guidelines.
Read AGENTS.md for the basic coding rules and guidelines.
Read REVIEW.md for the style and formatting rules you must enforce.
Read:
- .ai/<SLUG>/context.md
- .ai/<SLUG>/plan.md
- .ai/<SLUG>/implementation.md
Perform a code review focused on regressions, thread-safety, performance, and missing tests.
Perform a code review focused on:
- Style and formatting rules from REVIEW.md
- Regressions, thread-safety, performance, and missing tests
Write:
- .ai/<SLUG>/review.md

53
AGENTS.md
View File

@@ -125,58 +125,7 @@ if (user->isPremium()) {
// with the first message in each group.
```
**Empty line before closing brace:**
Always add an empty line before the closing brace of a class (after all private fields):
```cpp
// GOOD:
class MyClass {
public:
void foo();
private:
int _value = 0;
};
// BAD:
class MyClass {
public:
void foo();
private:
int _value = 0;
};
```
**Multi-line expressions — operators at the start of continuation lines:**
When splitting an expression across multiple lines, place operators (like `&&`, `||`, `;`, `+`, etc.) at the **beginning** of continuation lines, not at the end of the previous line. This makes it immediately obvious from the left edge whether a line is a continuation or new code.
```cpp
// BAD - continuation looks like scope code:
if (const auto &lottie = animation->lottie;
lottie && lottie->valid() && lottie->framesCount() > 1) {
lottie->animate([=] {
// GOOD - semicolon at start signals continuation:
if (const auto &lottie = animation->lottie
; lottie && lottie->valid() && lottie->framesCount() > 1) {
lottie->animate([=] {
// BAD - trailing && makes next line look like independent code:
if (veryLongExpression() &&
anotherLongExpression() &&
anotherOne()) {
doSomething();
// GOOD - leading && clearly marks continuation:
if (veryLongExpression()
&& anotherLongExpression()
&& anotherOne()) {
doSomething();
```
**Style and formatting rules** are in `REVIEW.md` — see that file for empty-line-before-closing-brace, operator placement in multi-line expressions, if-with-initializer, and other mechanical style rules.
**Use `auto` for type deduction:**

96
REVIEW.md Normal file
View File

@@ -0,0 +1,96 @@
# Code Review Style Guide
This file contains style and formatting rules that the review subagent must check and fix. These are mechanical issues that should always be caught during code review.
## Empty line before closing brace
Always add an empty line before the closing brace of a class (after all private fields):
```cpp
// BAD:
class MyClass {
public:
void foo();
private:
int _value = 0;
};
// GOOD:
class MyClass {
public:
void foo();
private:
int _value = 0;
};
```
## Multi-line expressions — operators at the start of continuation lines
When splitting an expression across multiple lines, place operators (like `&&`, `||`, `;`, `+`, etc.) at the **beginning** of continuation lines, not at the end of the previous line. This makes it immediately obvious from the left edge whether a line is a continuation or new code.
```cpp
// BAD - continuation looks like scope code:
if (const auto &lottie = animation->lottie;
lottie && lottie->valid() && lottie->framesCount() > 1) {
lottie->animate([=] {
// GOOD - semicolon at start signals continuation:
if (const auto &lottie = animation->lottie
; lottie && lottie->valid() && lottie->framesCount() > 1) {
lottie->animate([=] {
// BAD - trailing && makes next line look like independent code:
if (veryLongExpression() &&
anotherLongExpression() &&
anotherOne()) {
doSomething();
// GOOD - leading && clearly marks continuation:
if (veryLongExpression()
&& anotherLongExpression()
&& anotherOne()) {
doSomething();
```
## Minimize type checks — prefer direct cast over is + as
Don't check a type and then cast — just cast and check for null. `asUser()` already returns `nullptr` when the peer is not a user, so calling `isUser()` first is redundant. The same applies to `asChannel()`, `asChat()`, etc.
```cpp
// BAD - redundant isUser() check, then asUser():
if (peer && peer->isUser()) {
peer->asUser()->setNoForwardFlags(
// GOOD - just cast and null-check:
if (const auto user = peer->asUser()) {
user->setNoForwardFlags(
```
When you need a specific subtype, look up the specific subtype directly instead of loading a generic type and then casting:
```cpp
// BAD - loads generic peer, then casts:
if (const auto peer = session().data().peerLoaded(peerId)
; peer && peer->isUser()) {
peer->asUser()->setNoForwardFlags(
// GOOD - look up the specific subtype directly:
const auto userId = peerToUser(peerId);
if (const auto user = session().data().userLoaded(userId)) {
user->setNoForwardFlags(
```
Avoid C++17 `if` with initializer (`;` inside the condition) when the code can be written more clearly with simple nested `if` statements or by extracting the value beforehand:
```cpp
// BAD - complex if-with-initializer:
if (const auto peer = session().data().peerLoaded(peerId)
; peer && peer->isUser()) {
// GOOD - simple nested ifs when direct lookup isn't available:
if (const auto peer = session().data().peerLoaded(peerId)) {
if (const auto user = peer->asUser()) {
```