Don't require translation screenshots on description-only changes
* This is a follow-up to crrev.com/c/2328669 with further fine tuning
from findings in https://crbug.com/1103844#c22.
* The current translation screenshot presubmit check flags modified
messages:
1) if the message does not yet have a screenshot, or
2) if the message content itself changed.
* This CL changes the behavior to for modified messages check in this
order:
1) if the message content itself changed -> flag the message.
2) if the message meaning changed and there is neither an existing
screenshot nor is a new one added with the CL -> flag the message.
* Important cases to consider: messages that don't yet have a
screenshot and for which only the message meaning attribute changed
will require a screenshot, but messages for which only the message
meaning changed and which already have a screenshot do not require a
new screenshot. And messages of which only the description attribute
changed simply never require new screenshots at all.
Bug: 1103844
Change-Id: I0318e685b2182a455dccfb0630ce768f5c990a65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412727
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Rainhard Findling <rainhard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808789}
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 543843a..9800df3 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -5099,18 +5099,19 @@
removed_ids = old_ids - new_ids
modified_ids = set([])
for key in old_ids.intersection(new_ids):
- if (old_id_to_msg_map[key].FormatXml()
- != new_id_to_msg_map[key].FormatXml()):
- sha1_path = input_api.os_path.join(
- screenshots_dir, key + '.png.sha1')
- if sha1_path not in new_or_added_paths and \
- not input_api.os_path.exists(sha1_path):
- # This message does not yet have a screenshot. Require one.
- modified_ids.add(key)
- elif (old_id_to_msg_map[key].ContentsAsXml('', True)
+ if (old_id_to_msg_map[key].ContentsAsXml('', True)
!= new_id_to_msg_map[key].ContentsAsXml('', True)):
# The message content itself changed. Require an updated screenshot.
modified_ids.add(key)
+ elif old_id_to_msg_map[key].attrs['meaning'] != \
+ new_id_to_msg_map[key].attrs['meaning']:
+ # The message meaning changed. Ensure there is a screenshot for it.
+ sha1_path = input_api.os_path.join(screenshots_dir, key + '.png.sha1')
+ if sha1_path not in new_or_added_paths and not \
+ input_api.os_path.exists(sha1_path):
+ # There is neither a previous screenshot nor is a new one added now.
+ # Require a screenshot.
+ modified_ids.add(key)
if run_screenshot_check:
# Check the screenshot directory for .png files. Warn if there is any.
