[tools] Validate store metadata in pre-commit hook#12716
Conversation
Add a `files [--fix] FILE...` subcommand to check_store_metadata.py that validates only the supplied paths and dispatches each to the appropriate check based on its path. Both fdroid and google flavors are handled the same way, plus iphone/metadata/. With --fix, trailing whitespace is rstripped and written back so the file matches what the store will actually display. Wire this into the pre-commit hook: staged Play Store and iOS metadata files are passed through `files --fix`, files mutated by --fix are detected with `git diff --quiet` and re-staged, and the commit is blocked on validation failures (over-length, emoji, bad URL, etc.). Also raise the listings/release-notes.txt limit from 499 to 500 to match the documented store limit. The earlier 499 was a workaround for trailing newlines that --fix now removes; this also unblocks sl/release-notes.txt which currently has exactly 500 chars. Signed-off-by: Alexander Borsuk <me@alex.bio>
| 'release_notes.txt': 4000, | ||
| 'keywords.txt': 100, | ||
| } | ||
| IOS_URL_FILES = ('support_url.txt', 'marketing_url.txt', 'privacy_url.txt') |
There was a problem hiding this comment.
The limits here duplicate the inline constants in check_android() (lines 265–271) and check_ios() (lines 307–312). The existing code comment acknowledges this, but a single future edit to one set without updating the other will silently cause the files pre-commit path and the android/ios CI full-scan path to enforce different limits. Consider having check_android() and check_ios() consume these dicts directly to eliminate the second source of truth.
Generated by Claude Code
| if locale not in ANDROID_LOCALES: | ||
| return error(path, 'unsupported locale: {}', locale) | ||
| if name in ANDROID_LISTING_LIMITS: | ||
| return done(path, check_raw(path, ANDROID_LISTING_LIMITS[name], fix=fix)[0]) |
There was a problem hiding this comment.
check_raw() is called directly here without a try/except FileNotFoundError guard. check_text() (used by the full-scan subcommands) handles missing files gracefully. The pre-commit shell hook pre-filters with [ -f "$FILE" ] so the hook path is safe, but running check_store_metadata.py files path/does/not/exist.txt from the CLI will produce an unhandled traceback instead of a ❌ error message. The same applies to the check_url() and check_email() call-sites in check_path() below (lines 338 and 354).
Generated by Claude Code
| def check_text(path, max, optional=False): | ||
| with open(path, 'rb') as f: | ||
| raw = f.read() | ||
| text = raw.decode('utf-8').rstrip() |
There was a problem hiding this comment.
What if raw data contains invalid utf-8 characters?
Let's wrap in try-except and handle UnicodeDecodeError.
There was a problem hiding this comment.
What is the best way to handle UnicodeDecodeError if not to fail with that exception?
There was a problem hiding this comment.
try:
text = raw.decode('utf-8').rstrip()
except UnicodeDecodeError as e:
return error(path, "Unicode error: " +str(e)), ""Here str(e) would contain message:
'utf-8' codec can't decode byte 0xff in position 0: invalid start byte
| (ok, email) = check_raw(path, 500) | ||
| def check_email(path, fix=False): | ||
| (ok, email) = check_raw(path, 500, fix=fix) | ||
| ok = ok and email.find('@') != -1 and email.find('.') != -1 |
There was a problem hiding this comment.
| ok = ok and email.find('@') != -1 and email.find('.') != -1 | |
| ok = ok and '@' in email and '.' in email |
Add a
files [--fix] FILE...subcommand to check_store_metadata.py that validates only the supplied paths and dispatches each to the appropriate check based on its path. Both fdroid and google flavors are handled the same way, plus iphone/metadata/. With --fix, trailing whitespace is rstripped and written back so the file matches what the store will actually display.Wire this into the pre-commit hook: staged Play Store and iOS metadata files are passed through
files --fix, files mutated by --fix are detected withgit diff --quietand re-staged, and the commit is blocked on validation failures (over-length, emoji, bad URL, etc.).Also raise the listings/release-notes.txt limit from 499 to 500 to match the documented store limit. The earlier 499 was a workaround for trailing newlines that --fix now removes; this also unblocks sl/release-notes.txt which currently has exactly 500 chars.