-
Notifications
You must be signed in to change notification settings - Fork 10
Avoid adding ranges in update-bcd where current data already satisfies them #1068
Comments
Absolutely! Combining two arrays of support statements is surprisingly hard and there are lots of cases not handled, and some are handled poorly. I've documented some of what's wrong in #571. But this is a case I think must be possible to fix without changing the overall approach. Note that the reason a lot of weird changes like this are left is because we've mostly been picking the good stuff from the changes produced, and not gotten anywhere near burning down all proposed changes to find all issues like this. |
Note to self: this is happening for |
The previous logic for finding a `simpleStatement` considered anything with a `version_removed` to be non-simple, going down the code path of adding entries unless an identical one already existed. Instead, limit this case to when there were no existing statements, in which case there's no merging of entries to worry about. Defer dealing with multiple existing statements or multiple inferred statements beyond the simple case of no existing statements. The resulting edits suggest differ by quite a lot from the previous state, overall updating fewer files. Fixes #1068. #875 is the plan for catching differences in the data which cannot yet be fixed automatically.
The previous logic for finding a `simpleStatement` considered anything with a `version_removed` to be non-simple, going down the code path of adding entries unless an identical one already existed. Instead, limit this case to when there were no existing statements, in which case there's no merging of entries to worry about. Defer dealing with multiple existing statements or multiple inferred statements beyond the simple case of no existing statements. The resulting edits suggest differ by quite a lot from the previous state, overall updating fewer files. Fixes #1068. #875 is the plan for catching differences in the data which cannot yet be fixed automatically.
The previous logic for finding a `simpleStatement` considered anything with a `version_removed` to be non-simple, going down the code path of adding entries unless an identical one already existed. Instead, limit this case to when there were no existing statements, in which case there's no merging of entries to worry about. Defer dealing with multiple existing statements or multiple inferred statements beyond the simple case of no existing statements. The resulting edits suggest differ by quite a lot from the previous state, overall updating fewer files. Fixes #1068. #875 is the plan for catching differences in the data which cannot yet be fixed automatically.
This should no longer be happening after #1088. The script is now a bit more conservative, so there may also be correct edits no longer being made. The total amount of changes makes this hard to vet, but I didn't find anything concerning when sampling the diff before and after the change. |
e.g. don't do this:
The text was updated successfully, but these errors were encountered: