docs(i18n): Vietnamese translation for queryset rules (DOL001–DOL007) - #54
Conversation
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdded a Vietnamese rule catalog README and localized documentation for DOL001–DOL007, including rule metadata, incorrect/correct examples, QuickFix details, semantic caveats, and suppression or workspace configuration guidance. ChangesVietnamese rule documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/i18n/rules/vi/DOL002.md`:
- Line 5: Update the Vietnamese performance wording in the DOL002 documentation
so it states that counting matching rows is much more expensive, replacing the
phrase that implies counting is only slightly more costly. Preserve the
surrounding explanation and the recommendation to use `.exists()`.
In `@docs/i18n/rules/vi/DOL005.md`:
- Line 5: Update the DOL005 Vietnamese rule text to remove the claim that
combining filter(...).exclude(...) results in one fewer query-planner or
database operation. Frame the suggestion as improving condition
explicitness/readability, while preserving the existing warning that
exclude(...) and ~Q(...) can differ on multi-valued relations and that no
QuickFix is provided.
In `@docs/i18n/rules/vi/DOL006.md`:
- Line 5: Revise the DOL006 explanation to state that list(qs) eagerly evaluates
the queryset and allocates an additional list container, without claiming it
copies each model instance. Clarify that direct QuerySet iteration is not
automatically streaming and that .iterator() must be used explicitly, while
preserving the QuickFix guidance to remove list() and iterate directly.
In `@docs/i18n/rules/vi/README.md`:
- Around line 7-39: Add the three missing rule entries to the Vietnamese README
catalog so it contains all 19 rules targeted by the PR, using the same table
structure and metadata format as the existing entries. Alternatively, explicitly
revise the catalog scope statement if only 16 rules are intended, but ensure the
documented coverage matches the PR objective.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1bfb40d9-1a45-40bf-a04e-3edc99b558ab
📒 Files selected for processing (8)
docs/i18n/rules/vi/DOL001.mddocs/i18n/rules/vi/DOL002.mddocs/i18n/rules/vi/DOL003.mddocs/i18n/rules/vi/DOL004.mddocs/i18n/rules/vi/DOL005.mddocs/i18n/rules/vi/DOL006.mddocs/i18n/rules/vi/DOL007.mddocs/i18n/rules/vi/README.md
FROWNINGdev
left a comment
FROWNINGdev
left a comment
There was a problem hiding this comment.
Thanks for this — and for taking a family as a self-contained chunk, which is exactly the shape #52 was asking for.
I verified what I can verify without reading Vietnamese, and the seven translated pages hold up well:
- every code block in DOL001–DOL006 is byte-identical to the English source;
- DOL007's only difference is the two
#comments inside the sample, which is fine — those are prose, and nothing copy-pasteable or greppable changed; - the suppression line, the
djangoOrmLens.rulessettings key and the rule code appear in all seven pages, unchanged; - heading structure matches the English page one-for-one, and the severity/applicability line is translated in place rather than dropped;
- on DOL005 you kept the multi-valued-relation caveat — that
.exclude()and~Q()diverge across a M2M or reverse FK — intact and correct. That is the one paragraph on the page that would do real damage if it were flattened, so thank you for handling it carefully.
One blocker, and it is entirely in docs/i18n/rules/vi/README.md, not in the translations.
The index is a table of all 19 rules, but only 7 exist in vi/. Two consequences:
- Twelve links 404 —
DOL011–DOL032, plusnplusone.mdandmigrations.mdunder "Tài liệu bổ sung". A broken docs page is the specific thing that has cost this project before, so I would rather not land more of them. - The rows for the untranslated rules describe rules we do not have. The DOL001–DOL007 rows are correct — summaries, severity and applicability all match. The rest do not:
| Code | Row in this PR | The actual rule on main |
|---|---|---|
| DOL011 | add db_index=True to often-filtered FKs (hint) |
null=True on CharField/TextField (warning) |
| DOL012 | null=True on a string field (info) |
Model without __str__ (info) |
| DOL013 | missing related_name on FK (hint) |
ForeignKey without on_delete (error) |
| DOL014 | use get_or_create() over try/except (info) |
CharField without max_length (error) |
| DOL015 | ManyToManyField without through= (hint) |
TextField with max_length has no DB effect (hint) |
| DOL031 | do not call form.save() when is_valid() is False (error) |
render() with locals() as context (warning) |
| DOL032 | always call is_valid() before save() (error) |
fields = '__all__' in Meta (warning) |
DOL021/DOL022 name the right rule but mark applicability safe; both are suggestion.
Those are plausible Django lint rules — they are just not ours, and a reader who trusts the table would go looking for a related_name diagnostic that never fires, or would read DOL013 as a hint when it defaults to error. Please rebuild the index from docs/rules/README.md on main rather than from the rule codes.
What I would like instead: cut the index down to the seven rules this PR actually ships, and either drop the other sections or point them at the English pages (../../../rules/DOL011.md) so nothing dead-ends. Both are fine by me — your call.
Two smaller notes:
- I clarified an acceptance criterion on #52 after you opened this, so you could not have seen it: since there is no
docs/i18n/README.vi.md, do not treat translating the whole README as a prerequisite. Just linkdocs/i18n/rules/vi/README.mdfromdocs/rules/README.mdand I will wire up the language switcher myself. - The migration analyzer went from 15 rules to 16 in
py-1.6.0(conflicting_migration_leaves). Only relevant if you takemigrations.mdin a later chunk.
Fix the index and I will merge this. The translation work itself is good, and Vietnamese is a language we have no coverage in at all.
Both were surfaced by the automated review on #54, where a contributor had faithfully translated them into Vietnamese. The defect is in the English source, not in the translation. DOL005 claimed that collapsing `.filter().exclude()` into `.filter(Q(...) & ~Q(...))` buys "one pass for the query planner". It does not — Django compiles the chained form into a single query already, so the rewrite changes nothing about how many statements the database plans. The rule is a legibility hint and now says so. The multi-valued-relation caveat, which is the part of that page that actually prevents bugs, is unchanged. DOL006 claimed `list(qs)` "builds a second in-memory copy of every row". It does not copy the model instances; `_fetch_all()` builds them once and the list holds the same objects, so the real cost is the extra container plus the loss of laziness. The page also implied that dropping `list()` gives you streaming, which it does not — a plain queryset iteration still fills `_result_cache` end to end, and chunked reads need an explicit `.iterator()`. Reported via automated review on PR #54 (@RinZ27).
|
Follow-up on the automated review — I checked its three prose findings against the English source, and two of them are our bugs, not yours. You translated the English faithfully; the English was wrong. Both are now fixed on
Please re-translate those two paragraphs from On the third finding, DOL002: I think the bot misread it, and I would like your read as the native speaker. It flagged The index rebuild from my earlier review is still the one blocker. |
dab7598 to
1a27758
Compare
|
The three prose changes are right — DOL005 now frames it as explicitness rather than a planner win, DOL006 drops the copy claim and adds the The blocker from my review is still open, though, and it is the only thing between this and a merge. My earlier comment was too long and buried it, so here it is on its own:
Noting that the catalog contains 16 rules answers the bot's complaint about the count, but it does not stop those twelve links from 404ing for a reader — and the rows themselves still describe rules we do not have. Concretely, what I need: delete the four sections for rules this PR does not translate — "Định nghĩa Model", "Ngày giờ", "Forms / Views" and "Tài liệu bổ sung" — leaving the Queryset table plus the severity and applicability legends. Those rows are all verified correct. Then add one line saying the remaining rules are not translated yet and pointing at the English reference. That is the whole change: the file gets shorter, nothing else moves. If you would rather I just do it, say so — |
Signed-off-by: RinZ27 <222222878+RinZ27@users.noreply.github.com>
1a27758 to
3ec0bc6
Compare
FROWNINGdev
left a comment
FROWNINGdev
left a comment
There was a problem hiding this comment.
Index is right now — seven rows, seven files, every link resolves. I re-ran the check against 3ec0bc6 and there are no 404s left.
Re-verified the rest after your prose edits, since those touched the translated pages:
- code blocks in DOL001–DOL006 still byte-identical to the English source; DOL007's only difference remains the two translated
#comments, which is fine; - the severity/applicability line matches the English page on every rule, including the ones I spot-checked after the rewrite;
- suppression syntax and the
djangoOrmLens.ruleskey intact throughout.
Also confirmed this does not clash with a change I landed on main mid-review: I bumped the migration-rule count in docs/rules/README.md from 15 to 16 while your branch was open. Your branch never touched that line, so the merge keeps 16 — the two-dot diff makes it look like a revert, but it is not one.
Approving and merging. Thanks for the patience through three rounds of review, and for finding a documentation bug that had been wrong since the rules shipped.
Follow-ups I owed after merging #54, none of which belonged in a contributor's PR: - The language switcher on the English rule index now says which rules the Vietnamese set actually covers, so a reader knows before clicking that it is the queryset family rather than all 19 pages. Also drops a stray blank line the merge left behind. - The Vietnamese index gains a pointer to the English reference for the rules it does not cover. That note is in English on purpose: writing Vietnamese I do not speak is exactly what #52 tells contributors not to do, so it says so and invites a translation. - CHANGELOG records the translation and credits @RinZ27, plus the two documentation bugs the review of that PR uncovered in DOL005 and DOL006 — both wrong since the rules shipped.
|
Merged in
If you take another chunk, |
2 participants
Summary
Vietnamese (
vi) translations for the queryset rule group (DOL001–DOL007), following thedocs/i18n/layout convention. Partial coverage per the contributor note in #52 — the queryset group is the most commonly encountered family in day-to-day Django code so it made sense as the first chunk.Type of change
Test plan
Docs-only change. Verified all relative links between the translated files resolve correctly on GitHub. Code blocks, rule codes, option names, and suppression syntax are kept byte-identical to the English source — only explanatory prose is translated. Vietnamese is my native language; no machine translation used.
Checklist
cd cli && pytest -qfor Python,npm testfor TypeScript) and it is green## [Unreleased]mcp_server.pyand the relevant tests intest_mcp_server.py## Summaryabove and suggested a migration pathRelated issues / discussions
Refs #52