-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
🐛 Fixes issue in ObservableList while using Iterables. #942
🐛 Fixes issue in ObservableList while using Iterables. #942
Conversation
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.
Also need pubspec update, changelog and running the melos run set_version
command to update the lib/mobx.dart
file
@@ -336,7 +341,7 @@ class ObservableList<T> | |||
if (iterable.isNotEmpty) { | |||
final oldValues = _list.sublist(index, index + iterable.length); | |||
final newValues = iterable.toList(growable: false); |
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.
newValues
can be created first and then used to fetch the length
. That way the previous call on iterable.length
is not needed (which actually forces a complete walk of the iterable)
cfe5f81
to
d10b325
Compare
Hi @pavanpodila , I've made necessary changes in the PR But while running |
d10b325
to
558aed3
Compare
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.
thanks for fixing the melos script. We have one change in flutter_mobx due to a lagging version update. Can you please include a small changelog for that..might require reading the git log
flutter_mobx/lib/version.dart
Outdated
@@ -1,4 +1,4 @@ | |||
// Generated via set_version.dart. !!!DO NOT MODIFY BY HAND!!! | |||
|
|||
/// The current version as per `pubspec.yaml`. | |||
const version = '2.0.6+5'; |
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.
looks like this version was lagging behind and got updated once the set_version was run. Could you please include a changelog entry for this by reading the changes done in the past version.
Codecov Report
@@ Coverage Diff @@
## master #942 +/- ##
===========================================
+ Coverage 98.99% 100.00% +1.00%
===========================================
Files 57 6 -51
Lines 1990 100 -1890
===========================================
- Hits 1970 100 -1870
+ Misses 20 0 -20
Flags with carried forward coverage won't be shown. Click here to find out more. see 51 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
one of the test has failed with this change. pls see @ParthBaraiya |
9806ae9
to
e951de2
Compare
Looks like you have already merged a changes for this on master. |
This is fixed. |
@all-contributors add @ParthBaraiya for code, bug |
I've put up a pull request to add @ParthBaraiya! 🎉 |
Describe the changes proposed in this Pull Request.
This PR Fixes issue #941 .
Pull Request Checklist
pubspec.yaml
is updated.major
/minor
/patch
/patch-count
, depending on the complexity of changemelo run set_version
command from the root directory