Skip to content

Checks: Persist placeholder warnings to DB - #4499

Open
ayshushus wants to merge 1 commit into
mozilla:mainfrom
ayshushus:4364-string-warnings
Open

Checks: Persist placeholder warnings to DB#4499
ayshushus wants to merge 1 commit into
mozilla:mainfrom
ayshushus:4364-string-warnings

Conversation

@ayshushus

@ayshushus ayshushus commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fix #4364

Placeholder and element warnings from checks (specifically like Android) were being routed to pndbWarnings (non-DB) instead of pWarnings (stored), causing an issue if the translator "Saves anyway".

Split the warning bucket in custom.py to separate stored warnings from non-DB warnings.
Add regression guard.

@ayshushus ayshushus self-assigned this Sep 5, 2026
@ayshushus
ayshushus marked this pull request as ready for review September 7, 2026 05:25
@ayshushus
ayshushus requested a review from eemeli September 7, 2026 05:25

@eemeli eemeli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix itself looks fine, but there's a bit of unnecessary feature creep here.

format = cast(Resource.Format, entity.resource.format)
errors: list[str] = []
warnings: list[str] = []
ndb_warnings: list[str] = []

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be a little less cryptic and spend a few more characters here.

Suggested change
ndb_warnings: list[str] = []
non_db_warnings: list[str] = []

Comment thread pontoon/checks/utils.py
Comment on lines +61 to +64
if library not in NON_DB_LIBRARIES:
log.error(
f"Discarding failed checks from unknown library {library!r}: {messages}"
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit gratuitous. This never happens, and we're working on refactoring the linting code into moz-l10n, so the changes here are quite unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strings with warnings are not marked as such in the UI

2 participants