Repository navigation
Project config: preserve resources and localized files - #4571
Conversation
mathjazz
left a comment
There was a problem hiding this comment.
Thanks for the patch! Left a few comments inline.
| removed_paths = { | ||
| get_db_path(paths, join(checkout.path, co_path)) for co_path in checkout.removed | ||
| } | ||
| if isinstance(paths, L10nConfigPaths): |
There was a problem hiding this comment.
Marking entities obsolete here can't be undone later. When a path comes back, update_resources() un-obsoletes the resource but not its entities, so it creates a second copy of every Entity, and the old translations, comments and reviews stay stuck on the hidden ones.
There was a problem hiding this comment.
Is this effectively #4128? Based on your comment there, I'm applying a content (value & properties) match that is LIFO/newest copy. Also adding a "Restored" log.
| get_db_path(paths, join(checkout.path, co_path)) for co_path in checkout.removed | ||
| } | ||
| if isinstance(paths, L10nConfigPaths): | ||
| ref_db_paths = {get_db_path(paths, ref_path) for ref_path in paths.ref_paths} |
There was a problem hiding this comment.
ref_paths is just whatever the glob matched this run, so a typo in reference (or a source folder moved one commit before the config is updated) makes it empty and wipes out the whole project in one sync.
Shall we refuse to obsolete when the list covers all or nearly all resources?
| current = set(project.resources.current().values_list("path", flat=True)) | ||
| for ref_path in paths.ref_paths: | ||
| if ( | ||
| relpath(ref_path, paths.ref_root) in current |
There was a problem hiding this comment.
This skips references we already track, so if only the l10n target moves (e.g. {locale}/a.ftl → l10n/{locale}/a.ftl), the files at the new location never get added to target.changed and their translations aren't picked up.
eemeli
left a comment
There was a problem hiding this comment.
A partial review, happy to have Matjaž take the lead here.
Co-authored-by: Eemeli Aro <eemeli@gmail.com>
mathjazz
left a comment
There was a problem hiding this comment.
This looks good, nice work!
Co-authored-by: Eemeli Aro <eemeli@gmail.com>
Co-authored-by: Eemeli Aro <eemeli@gmail.com>
Fix #2844
Fix #4385
paths.ref_pathsnow go throughmark_as_obsolete().