Move to EasyList#185
Conversation
…llection and update tests
henrikjacobsenfys
added
[scope] enhancement
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #185 +/- ##
===========================================
+ Coverage 98.18% 98.42% +0.23%
===========================================
Files 47 50 +3
Lines 3142 3175 +33
Branches 570 576 +6
===========================================
+ Hits 3085 3125 +40
+ Misses 35 26 -9
- Partials 22 24 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| self._validate_type(value) | ||
| self._check_name_unique(value) | ||
| super().insert(index, value) | ||
| value.lock_name() |
There was a problem hiding this comment.
ProtectedType_ is bound to NewBase, but NewBase doesn't have a lock_name method. I see it's from a name_mixin, but then i'd expect typevar must be different. Doesn't ruff complain about this call?
There was a problem hiding this comment.
Good point! The protected type should be my base classes, not the easyscience ones.
There was a problem hiding this comment.
(But to answer your question: no, Ruff did not complain)
rozyczko
left a comment
rozyczko
left a comment
There was a problem hiding this comment.
Good changes!
Please consider adding the duplicate check to append, then this PR is ready to merge.
| self._validate_type(value) | ||
| super().append(value) |
There was a problem hiding this comment.
this method, as opposed to say, insert, doesn't check for duplicate names.
There was a problem hiding this comment.
I wonder if this is needed, since I believe all methods in the end call insert to do the insertion. The tests will show when I get around to writing them :)
There was a problem hiding this comment.
Update: it's not needed :)
3 participants
It's missing a few tests, and perhaps moving a few tests around. However, the missing tests are really simple and I therefore deem it ready for review.
See #186 for the philosophy behind this.
Closes #117