-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(disk-transform): prepare for qcow disks #8228
base: master
Are you sure you want to change the base?
Conversation
c88d74c
to
52f23d0
Compare
52f23d0
to
b165bf9
Compare
import assert from 'node:assert' | ||
|
||
export class ForkableIterator<T> { | ||
#forks = new Set<ForkedIterator<T>>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, if you want to benefit from classes, you should also use private/public/protected/static which go with them, instead of #.
By using #, you only have the choice between public and private.
If we do class inheritance, the properties/methods will either be inaccessible to inheriting classes (private), or accessible and modifiable from everywhere (everything is public = more encapsulation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#
is better thanprivate
because it is enforced at runtimeprotected
does not have an equivalent in ECMAScript, I don't see any issues with using itstatic
also exists in ECMAScriptpublic
is the default when neitherprivate
norprotected
, I don't see what this annotation brings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"# is better than private because it is enforced at runtime"
TS throws an error if you write code outside the class that accesses a private property or method. So you can't write code that accesses it from outside, it won't even reach runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TypeScript does the same with private fields: https://www.typescriptlang.org/play/?#code/MYGwhgzhAEDC0G8BQ1XQMQDswFsCmAXNBAC4BOAlpgOZIprAD2mpZArsCY2QBTb5FWVagEpE9NGhIALChAB0WXHmgBeaPzwToAXyR6kTFiWiM1GvAHc4PAOQAzRo1si6R0huXnGizUA
But private fields are standard ECMAScript and are enforced even types are stripped.
Description
the goal of this PR is to centralize all the logic that transform disks in a dedicated module. The typescript is fully contained in @xen-orchestra/disk-transform
Instantiate a portable format from a vhd on an remote, and then write it to another remote
Future PRs will add more sources ( xapi, nbd, cbt, vhd stream, vmdk, ova) , and target (xva, vhd stream )
Checklist
Fixes #007
,See xoa-support#42
,See https://...
)Introduced by
CHANGELOG.unreleased.md
Review process
Notes: