-
Notifications
You must be signed in to change notification settings - Fork 276
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
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.
ccb1733
to
43d3670
Compare
incremental backups works with or without cbt / nbd |
388bdad
to
dc391b7
Compare
mirror backups works |
1ef9b3f
to
a4a6f15
Compare
e9dcb1d
to
bcd177d
Compare
todo :
next step : v2v |
when using sublasses the behavuour was not predictable use more explicit code
ebb8f33
to
a5494d7
Compare
Description
the goal of this PR is to define a central point for all disk content transformation, and to port existing backup features
Futures PR will add
Checklist
Fixes #007
,See xoa-support#42
,See https://...
)Introduced by
CHANGELOG.unreleased.md
Review process
Notes: