Ticket #275 (closed task: fixed)
Code Review
| Reported by: | Joel | Owned by: | Joel |
|---|---|---|---|
| Priority: | major | Milestone: | Eraser 6.1/6.2 |
| Component: | Core | Version: | |
| Keywords: | Cc: | ||
| Processor Architecture: | Blocked By: | #276, #284, #307, #309, #320 | |
| Blocking: | #262 | Operating System: |
Description (last modified by Joel) (diff)
Find a way to return the status of the erase from the DirectExecutor? (like calling Invoke)Throw exceptions only when really necessary (such as in the Unused Space erasure code, maybe?)IDisposable.Dispose is supposed to be reentrantFix all MessageBox.Show calls to include RightAlign and the RtlReading members of the MessageBoxOptions enumeration. http://msdn.microsoft.com/en-us/library/ms182191.aspx
From #284:
- Extract the functions in the following files to discrete classes:
- Place functions in the SafeNativeMethods class for those which are "safe"
Blocking
| Id | Summary | Milestone |
|---|---|---|
| #275 | └ Code Review | Eraser 6.1/6.2 |
| #262 | └ Localise the Util.Native and Util libraries | Eraser 6.1/6.2 |
Blocked by
| Id | Summary | Milestone |
|---|---|---|
| #275 | └ Code Review | Eraser 6.1/6.2 |
| #320 | └ Logging improvements | Eraser 6.1/6.2 |
| #300 | └ Tasks - Information log entries for start / end | Eraser 6.1/6.2 |
| #284 | └ Eraser.Util rewrite | Eraser 6.1/6.2 |
| #272 | └ Remove IsFat12 from Fat12Or16Api | Eraser 6.1/6.2 |
| #90 | └ Generic speed meter | Eraser 6.1/6.2 |
| #316 | └ Eraser causes lag when running | Eraser 6.0.7 |
| #317 | └ Merge the BlackBox branch | Eraser 6.1/6.2 |
| #307 | └ Eraser Updater restructuring | Eraser 6.1/6.2 |
| #308 | └ Upgrade BlackBox to use the ProgressManager classes | Eraser 6.1/6.2 |
| #317 | └ Merge the BlackBox branch | Eraser 6.1/6.2 |
| #276 | └ Implement Eraser Registrar Interface | Eraser 6.1/6.2 |
| #309 | └ Remove the SettingsCompatibility classes | Eraser 6.1/6.2 |
Change History
comment:1 Changed 3 years ago by Joel
- Define the ERROR_* constants from Win32 instead of using hardcoded numbers for readability
- Perhaps write a function to replace the ExceptionFromHRESULT marshal function - the Util assembly usually throws strange exceptions
- Better Opportunist Command Line program behaviour
- Rewrite the Log viewer form!
- Rewrite the Updater/BlackBox? uploader -- unify them (Factor out the MultipartFormDataBuilder? class)
- Erasure methods should be based on an interface: IErasureMethod, IChainedErasureMethod
- Guid property of all Erasure methods should be static; implement the Guid property as an interface member instead
- Find a way to return the status of the erase from the DirectExecutor? (like calling Invoke)
- Replace all the new EventHandler?<X> calls - no need for that
- IList<T> vs. ICollection<T>?
- Combine FileSystem?.DeleteFile? and FileSystem?.DeleteFolder?
- Use System.Func generics instead of out own types of delegates
comment:7 Changed 3 years ago by Joel
Move helper classes like the ProgressManager? classes in #90 to the Util library.
comment:8 Changed 3 years ago by Joel
The updater classes are really messy! I'll need to restructure the code somehow.
comment:12 Changed 3 years ago by Joel
- Blocked By 90 added
comment:15 Changed 3 years ago by Joel
- Description modified (diff)
The System.Func generics don't really help -- especially since we are defining delegates of a particular nature (usually event handling)
comment:16 Changed 3 years ago by Joel
- Description modified (diff)
comment:17 Changed 3 years ago by Joel
- Description modified (diff)
Log viewer form reviewed in r1526
comment:18 Changed 3 years ago by Joel
- Description modified (diff)
"new EventHandler?" was removed from Eraser in r1527
comment:19 Changed 3 years ago by Joel
- Description modified (diff)
ICollections replaced with ILists, where applicable in r1528
comment:20 Changed 3 years ago by Joel
- Description modified (diff)
FileSystem.DeleteFile and FileSystem.DeleteFolder seems to be able to be combined only in our case in WindowsFileSystem, so combine it there first. Fixed in r1529
comment:24 Changed 3 years ago by Joel
- Description modified (diff)
- Blocked By 317 removed
These belong in #307
comment:34 Changed 3 years ago by Joel
- Description modified (diff)
Exceptions generally reviewed in r1767; exceptions are thrown only when we can't do anything about the error. Also, developer-visible exceptions are now non-localisable strings; user-visible exceptions should be changed to log messages.
comment:41 Changed 3 years ago by Joel
- Description modified (diff)
The design doesn't seem to work too well since we have the notion of queued tasks.
comment:43 Changed 3 years ago by Joel
- Description modified (diff)
The rest aren't important now, I'll prepare to close this ticket.
comment:44 Changed 3 years ago by Joel
- Status changed from accepted to closed
- Resolution set to fixed
Completed in r1803
