Conversation
feat: adding records to browser
feat: displaying history on tree table
feat: open history when adding a file
feat: DSSpy button & start/stop buttons on toolbar
fix: using now only one instance of browser refactor: renaming package
fix: error when opening timeline with not enough data
fix: timer that was not counting the seconds refactor: DSRecordBrowserPresenter => DSRecordBrowser refactor: some methods from browser going into the browser class doc: updating the documentation for browser
Julesboul
requested review from
Julesboul and
StevenCostiou
and removed request for
StevenCostiou
Julesboul
left a comment
Julesboul
left a comment
There was a problem hiding this comment.
Don't be affraid with the number of comments: many are repeated and it is common in reviews!
In general, this is a good code and this browser will be a great feature to add.
Pay attention to segmentation: the more your code is segmented, the best it is for testing and finding bugs.
feat: adding more tests for each browser feature refactor: removing ColoredRecord class; refactoring comment: adding more comments where needed
Julesboul
left a comment
Julesboul
left a comment
There was a problem hiding this comment.
That's good to see more tests and the way of generating a testing file is really better than before!
There are still few things to change and some careless mistakes.
test: adding more tests for timerWindow | generating random records for tests refactor: recorderWindow -> timerWindow comment: adding comments where needed
refactor: getRecordColorAssociationsFrom now uses a collection of associations
Julesboul
left a comment
Julesboul
left a comment
There was a problem hiding this comment.
Good job in general, just try to make your tests on the browser more simple: with less algorithms for the setup and building more things "by hand".
| { #category : 'accessing' } | ||
| DSAbstractEventRecord class >> getLeafSubClasses [ | ||
| "Returns a list of all non-abstract subclasses, representing all record classes." | ||
|
|
||
| | classes | | ||
| classes := OrderedCollection new. | ||
| self subclasses do: [ :subclass | | ||
| subclass subclasses isEmpty | ||
| ifTrue: [ classes add: subclass ] | ||
| ifFalse: [ | ||
| subclass = DSDoItRecord ifTrue: [ classes add: subclass ]. | ||
| classes addAll: subclass getLeafSubClasses ] ]. | ||
|
|
||
| ^ classes | ||
| ] |
There was a problem hiding this comment.
Is it possible to add this class method in the Browser package? And so to add a test?
You can create a testing class for Records in the Debugging Spy package, there is no unit tests on records for now but I will work on this later.
Julesboul
left a comment
Julesboul
left a comment
There was a problem hiding this comment.
Some very little things and after that it should be good !
2 participants
No description provided.