<doc>[docs]: <description#4116
Open
ZStack-Robot wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough在 SDK 的 PrimaryStorageMigrateVmAction 中：将 vmInstanceUuid 标记为 变更存储迁移API参数更新
Mermaid 序列图（本变更为简单参数定义更新，无需绘制序列图） 代码审查工作量估计🎯 2 (Simple) | ⏱️ ~10 分钟 庆祝诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 5
🤖 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/modules/core/pages/vm-migrate-impl-assessment.md`:
- Line 113: 当前文档中存在一个不带语言标记的围栏代码块（```），触发 markdownlint
MD040；请在该代码块的起始反引号后添加合适的语言标记（例如 `text` 或其他最贴切的语言标识），确保渲染一致性并消除 MD040 警告。
- Line 11: The document's Flow counts are inconsistent: reconcile the total Flow
count for handle(PrimaryStorageMigrateVmMsg) so the top summary, the table
entries (`#1`~`#9`), and later references like "5/8 个 Flow" all use the same number;
update either the header (currently "8 个 Flow") or the table (currently listing
`#1`~`#9`) and then change downstream mentions (including the occurrences around
lines 21, 33 and 271) so they consistently state the chosen total and any
derived fractions (e.g., "5/9" if you keep 9) and ensure the note about "5/8 个
Flow" is corrected to the matching numerator/denominator.
In `@docs/modules/core/pages/vm-migrate-volume-specs.md`:
- Line 232: The doc currently contradicts itself about
APIPrimaryStorageMigrateVmMsg: Line 232 says the old fields are kept without
deprecation, while lines 13/31/66 state they are marked with `@Deprecated` for
backward compatibility; decide on one policy and make the text consistent:
either (A) explicitly state that old fields are retained and NOT deprecated, and
update the `@Deprecated` mentions to removed, or (B) state that old fields are
marked with `@Deprecated` for compatibility and update Line 232 to reflect
deprecation; ensure references to APIPrimaryStorageMigrateVmMsg and
volumeMigrationSpecs consistently describe the chosen approach and that
`@Deprecated` is used only where intended.
- Around line 123-137: The fenced code block in vm-migrate-volume-specs.md lacks
a language tag (causing MD040); update the opening fence to include a language
identifier (e.g., change ``` to ```text or ```mermaid) for the block that starts
with "APIPrimaryStorageMigrateVmMsg" and contains the
FlowChain/RootMigrationFlow/DataVolumeMigrationFlow diagram so the linter
recognizes the fence language.
In `@docs/modules/core/pages/vm-migrate-with-snapshots.md`:
- Around line 21-35: The fenced code block starting at the snippet showing
APIPrimaryStorageMigrateVmMsg.withSnapshots /
PrimaryStorageMigrateVmMsg.withSnapshots should be annotated with a language to
satisfy MD040; update the triple-backtick fence to specify `text` so the block
is treated as plain text (affecting the section that mentions
APIPrimaryStorageMigrateVmMsg.withSnapshots,
PrimaryStorageMigrateVmMsg.withSnapshots, PrimaryStorageLiveMigrateVmMsg,
PrimaryStorageOfflineMigrateVmMsg,
MigrateVolumeOnPrimaryStorageMsg.migrateWithSnapshots and
StorageMigrationConstant.WITH_SNAPSHOTS).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 105c5719-157b-43e8-9aeb-90838b8c90bb
📒 Files selected for processing (3)
docs/modules/core/pages/vm-migrate-impl-assessment.mddocs/modules/core/pages/vm-migrate-volume-specs.mddocs/modules/core/pages/vm-migrate-with-snapshots.md
MatheMatrix
force-pushed
the
sync/tao.gan/live-migrate-data-volumes-ZSV-12280@@2
branch
from
9f47e6b to
c5182cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
docs/modules/core/pages/vm-migrate-with-snapshots.md (1)
21-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win补充 fenced code block 语言声明。
Line 21 的代码块未声明语言，建议改为
```text以消除 MD040。🤖 Prompt for 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. In `@docs/modules/core/pages/vm-migrate-with-snapshots.md` around lines 21 - 35, The fenced code block showing the withSnapshots migration flow (containing symbols like APIPrimaryStorageMigrateVmMsg.withSnapshots, PrimaryStorageMigrateVmMsg.withSnapshots, PrimaryStorageLiveMigrateVmMsg, PrimaryStorageOfflineMigrateVmMsg, MigrateVolumeOnPrimaryStorageMsg.migrateWithSnapshots, and StorageMigrationConstant.WITH_SNAPSHOTS) is missing a language specifier; update that markdown fence to declare the language as text (i.e., replace the opening ``` with ```text) so the linter rule MD040 is satisfied.docs/modules/core/pages/vm-migrate-volume-specs.md (2)
123-137:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win补充代码块语言标识以消除 MD040。
Line 123 的 fenced code block 仍未声明语言，建议改为
```text（或实际语法对应语言），避免 markdownlint 持续告警。🤖 Prompt for 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. In `@docs/modules/core/pages/vm-migrate-volume-specs.md` around lines 123 - 137, The fenced code block showing the flow (starting with APIPrimaryStorageMigrateVmMsg → Interceptor.normalize → StorageMigrationBase.handle ... APIPrimaryStorageMigrateVmEvent) lacks a language tag and triggers MD040; edit that block to add a language identifier such as ```text (or the appropriate language) before the block so markdownlint stops flagging it; locate the block containing APIPrimaryStorageMigrateVmMsg, Interceptor.normalize, StorageMigrationBase.handle, chainSubmit, FlowChain, RootMigrationFlow, DataVolumeMigrationFlow and MigrateVolumeOnPrimaryStorageMsg and prepend the language token to the opening backticks.
232-232:⚠️ Potential issue | 🟠 Major | ⚡ Quick win统一
@Deprecated策略口径，避免前后矛盾。Line 232“老字段保留不弃用”与 Line 13/31/66“标记
@Deprecated”冲突。请统一为单一策略，避免对 API 契约与实施步骤产生歧义。🤖 Prompt for 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. In `@docs/modules/core/pages/vm-migrate-volume-specs.md` at line 232, The documentation has conflicting deprecation guidance: Line 232 says keep the old field without deprecating, while Lines 13/31/66 state to mark fields with `@Deprecated`; resolve by choosing one consistent strategy for APIPrimaryStorageMigrateVmMsg—either (A) keep the old field and explicitly remove any `@Deprecated` markers and update all mentions to state "old field retained (not deprecated)," or (B) mark the old field as `@Deprecated` everywhere and document migration to volumeMigrationSpecs, updating all instances (Lines 13/31/66/232) to the same wording; ensure the chosen policy is applied consistently to APIPrimaryStorageMigrateVmMsg, volumeMigrationSpecs, and any referenced fields in this doc.
🤖 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/modules/core/pages/vm-migrate-capability-matrix.md`:
- Around line 32-42: The fenced code block starting at the shown snippet lacks a
language tag which triggers MD040; update the markdown by adding a language
identifier (e.g., use ```text) to the code fence that contains the pseudo-code
loop (the block showing flow with metric.isCapable, metric.isSupportLive,
metric.isSupportOffline, metric.isSupportWithDataVolume,
metric.isSupportWithSnapshot and the final throw) so the linter recognizes it as
a text/code block and MD040 is satisfied.
- Around line 190-207: The fenced code block that starts with
"APIPrimaryStorageMigrateVmMsg" in vm-migrate-capability-matrix.md lacks a
language identifier; update the opening fence from ``` to ```text (or replace
with the appropriate diagram language such as mermaid if you plan to convert to
diagram syntax) so the block is explicitly marked as plain text/diagram and
renders correctly.
---
Duplicate comments:
In `@docs/modules/core/pages/vm-migrate-volume-specs.md`:
- Around line 123-137: The fenced code block showing the flow (starting with
APIPrimaryStorageMigrateVmMsg → Interceptor.normalize →
StorageMigrationBase.handle ... APIPrimaryStorageMigrateVmEvent) lacks a
language tag and triggers MD040; edit that block to add a language identifier
such as ```text (or the appropriate language) before the block so markdownlint
stops flagging it; locate the block containing APIPrimaryStorageMigrateVmMsg,
Interceptor.normalize, StorageMigrationBase.handle, chainSubmit, FlowChain,
RootMigrationFlow, DataVolumeMigrationFlow and MigrateVolumeOnPrimaryStorageMsg
and prepend the language token to the opening backticks.
- Line 232: The documentation has conflicting deprecation guidance: Line 232
says keep the old field without deprecating, while Lines 13/31/66 state to mark
fields with `@Deprecated`; resolve by choosing one consistent strategy for
APIPrimaryStorageMigrateVmMsg—either (A) keep the old field and explicitly
remove any `@Deprecated` markers and update all mentions to state "old field
retained (not deprecated)," or (B) mark the old field as `@Deprecated` everywhere
and document migration to volumeMigrationSpecs, updating all instances (Lines
13/31/66/232) to the same wording; ensure the chosen policy is applied
consistently to APIPrimaryStorageMigrateVmMsg, volumeMigrationSpecs, and any
referenced fields in this doc.
In `@docs/modules/core/pages/vm-migrate-with-snapshots.md`:
- Around line 21-35: The fenced code block showing the withSnapshots migration
flow (containing symbols like APIPrimaryStorageMigrateVmMsg.withSnapshots,
PrimaryStorageMigrateVmMsg.withSnapshots, PrimaryStorageLiveMigrateVmMsg,
PrimaryStorageOfflineMigrateVmMsg,
MigrateVolumeOnPrimaryStorageMsg.migrateWithSnapshots, and
StorageMigrationConstant.WITH_SNAPSHOTS) is missing a language specifier; update
that markdown fence to declare the language as text (i.e., replace the opening
``` with ```text) so the linter rule MD040 is satisfied.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b2ee4d62-b936-41e9-9b6e-40d58e007968
📒 Files selected for processing (4)
docs/modules/core/pages/vm-migrate-capability-matrix.mddocs/modules/core/pages/vm-migrate-impl-assessment.mddocs/modules/core/pages/vm-migrate-volume-specs.mddocs/modules/core/pages/vm-migrate-with-snapshots.md
MatheMatrix
force-pushed
the
sync/tao.gan/live-migrate-data-volumes-ZSV-12280@@2
branch
8 times, most recently
from
205e2b3 to
ee1f5af
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@sdk/src/main/java/org/zstack/sdk/PrimaryStorageMigrateVmAction.java`:
- Around line 31-33: Add a Javadoc block to the deprecated field
dstPrimaryStorageUuid describing why it was deprecated, the recommended
replacement (volumeMigrationSpecs), and the planned removal version (if known);
update the comment above the field dstPrimaryStorageUuid to state the
deprecation reason, point callers to use volumeMigrationSpecs instead, and
include an estimated removal version or "TBD" if unknown.
- Around line 53-54: The field volumeMigrationSpecs in class
PrimaryStorageMigrateVmAction is declared as a raw java.util.List; change it to
a parameterized type (e.g. java.util.List<VolumeMigrationSpec> or the actual
spec class used for "卷级迁移目标") to restore type safety and remove unchecked
warnings, update the import or fully-qualify the generic type as needed, and
ensure any usages (constructors, getters/setters, serializers) referencing
volumeMigrationSpecs are updated to the new generic type to match the change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a28245a5-5bb2-4426-b173-5c670433a603
📒 Files selected for processing (1)
sdk/src/main/java/org/zstack/sdk/PrimaryStorageMigrateVmAction.java
Comment on lines
+31
to
33
| @Deprecated | ||
| @Param(required = true, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String dstPrimaryStorageUuid; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
@Deprecated 字段缺少 Javadoc 注释
根据编码规范第 3 条，接口和公开方法必须配有有效的 Javadoc 注释。当标记字段为 @Deprecated 时，应添加 Javadoc 注释说明：
- 废弃的原因
- 建议使用的替代方案（根据 PR 描述，应该是新增的
volumeMigrationSpecs字段） - 预计移除的版本（如适用）
📝 建议添加 Javadoc 注释
+ /**
+ * `@deprecated` 使用 {`@link` `#volumeMigrationSpecs`} 替代，以支持卷级别的迁移目标指定
+ */
`@Deprecated`
`@Param`(required = true, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
public java.lang.String dstPrimaryStorageUuid;As per coding guidelines: 代码应尽量做到自解释，对于较长的注释，需要仔细校对并随代码更新，确保内容正确。接口方法必须配有有效的 Javadoc 注释。
🤖 Prompt for 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.
In `@sdk/src/main/java/org/zstack/sdk/PrimaryStorageMigrateVmAction.java` around
lines 31 - 33, Add a Javadoc block to the deprecated field dstPrimaryStorageUuid
describing why it was deprecated, the recommended replacement
(volumeMigrationSpecs), and the planned removal version (if known); update the
comment above the field dstPrimaryStorageUuid to state the deprecation reason,
point callers to use volumeMigrationSpecs instead, and include an estimated
removal version or "TBD" if unknown.
Comment on lines
+53
to
+54
| @Param(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.util.List volumeMigrationSpecs; |
There was a problem hiding this comment.
使用原始 List 类型降低了类型安全性
字段 volumeMigrationSpecs 声明为原始类型 java.util.List，没有指定泛型参数。这会导致：
- 编译器无法进行类型检查，可能在运行时抛出
ClassCastException - 代码可读性降低，开发者无法从签名了解列表元素的具体类型
- 产生编译器警告（unchecked warnings）
根据 PR 描述，该字段用于 "指定卷级迁移目标"，应该有明确的元素类型定义。请指定具体的泛型参数（例如 java.util.List<VolumeMigrationSpec> 或其他适当的类型）。
♻️ 建议指定泛型类型
`@Param`(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
- public java.util.List volumeMigrationSpecs;
+ public java.util.List<VolumeMigrationSpec> volumeMigrationSpecs;注：请根据实际的卷迁移规范类名调整泛型参数。
As per coding guidelines: 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途。
🤖 Prompt for 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.
In `@sdk/src/main/java/org/zstack/sdk/PrimaryStorageMigrateVmAction.java` around
lines 53 - 54, The field volumeMigrationSpecs in class
PrimaryStorageMigrateVmAction is declared as a raw java.util.List; change it to
a parameterized type (e.g. java.util.List<VolumeMigrationSpec> or the actual
spec class used for "卷级迁移目标") to restore type safety and remove unchecked
warnings, update the import or fully-qualify the generic type as needed, and
ensure any usages (constructors, getters/setters, serializers) referencing
volumeMigrationSpecs are updated to the new generic type to match the change.
Resolves: ZSV-12280 Change-Id: If1b25061ef00e849ad1b7f1bc6de897432baea41
MatheMatrix
force-pushed
the
sync/tao.gan/live-migrate-data-volumes-ZSV-12280@@2
branch
from
ee1f5af to
614b691
Compare
|
Actionable comments posted: 0 |
Collaborator
Author
|
Comment from yaohua.wu: Review: MR !10014 — ZSV-12280Background
P0 — Critical无。 P1 — Warning
P2 — Suggestion
Coverage
Verdict: REVISION_REQUIRED本仓自身无需改动代码（#2 维持原样、#3 仅标题），但 #1 的 🤖 Robot Reviewer |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: ZSV-12280
Change-Id: I63616c67617a64706662726775727a6c6e6e647a
sync from gitlab !10014