Clea-up l10n::GetParents in-favor of LanguageTag::GetParentTag [chromium/src : main]

0 views
Skip to first unread message

Tim (Gerrit)

unread,
Jul 24, 2026, 6:17:40 PM (10 hours ago) Jul 24
to Danilo Tedeschi, Chromium LUCI CQ, Nicholas Verne, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, extension...@chromium.org, chromium-a...@chromium.org, jshin...@chromium.org, crost...@chromium.org
Attention needed from Danilo Tedeschi and Nicholas Verne

Tim added 2 comments

Patchset-level comments
File-level comment, Patchset 12 (Latest):
Tim . resolved

High level question on this one: I like the move to LanguageTag::GetParentTag, but I feel we've now got two very similar loops in order to get the full chain of tags for both cases. Would it make more sense to keep l10n::GetParents and put the for loop in there instead? Then we don't even need to update the callers and can keep the testing. Alternatively maybe adding a LanguageTag::GetParentTags (plural) for this behavior?

File extensions/common/extension_resource_unittest.cc
Line 193, Patchset 12 (Latest): .value_or(base::i18n::GetKnownLanguageTag("und"));
Tim . unresolved

Why are we only doing the fallback "und" behavior on this version and not the one in the other file?

Open in Gerrit

Related details

Attention is currently required from:
  • Danilo Tedeschi
  • Nicholas Verne
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id2737ea3a8c819b27648900faf1e44fcddac7085
Gerrit-Change-Number: 8120546
Gerrit-PatchSet: 12
Gerrit-Owner: Danilo Tedeschi <da...@google.com>
Gerrit-Reviewer: Danilo Tedeschi <da...@google.com>
Gerrit-Reviewer: Nicholas Verne <nve...@chromium.org>
Gerrit-Reviewer: Tim <tjud...@chromium.org>
Gerrit-Attention: Danilo Tedeschi <da...@google.com>
Gerrit-Attention: Nicholas Verne <nve...@chromium.org>
Gerrit-Comment-Date: Fri, 24 Jul 2026 22:17:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Danilo Tedeschi (Gerrit)

unread,
Jul 24, 2026, 6:57:56 PM (9 hours ago) Jul 24
to Tim, Chromium LUCI CQ, Nicholas Verne, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, extension...@chromium.org, chromium-a...@chromium.org, jshin...@chromium.org, crost...@chromium.org
Attention needed from Nicholas Verne and Tim

Danilo Tedeschi added 1 comment

File extensions/common/extension_resource_unittest.cc
Line 193, Patchset 12: .value_or(base::i18n::GetKnownLanguageTag("und"));
Tim . resolved

Why are we only doing the fallback "und" behavior on this version and not the one in the other file?

Danilo Tedeschi

makes sense, fixed the code. Thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Verne
  • Tim
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Id2737ea3a8c819b27648900faf1e44fcddac7085
    Gerrit-Change-Number: 8120546
    Gerrit-PatchSet: 13
    Gerrit-Owner: Danilo Tedeschi <da...@google.com>
    Gerrit-Reviewer: Danilo Tedeschi <da...@google.com>
    Gerrit-Reviewer: Nicholas Verne <nve...@chromium.org>
    Gerrit-Reviewer: Tim <tjud...@chromium.org>
    Gerrit-Attention: Tim <tjud...@chromium.org>
    Gerrit-Attention: Nicholas Verne <nve...@chromium.org>
    Gerrit-Comment-Date: Fri, 24 Jul 2026 22:57:46 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Tim <tjud...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Tim (Gerrit)

    unread,
    Jul 24, 2026, 7:16:09 PM (9 hours ago) Jul 24
    to Danilo Tedeschi, Chromium LUCI CQ, Nicholas Verne, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, extension...@chromium.org, chromium-a...@chromium.org, jshin...@chromium.org, crost...@chromium.org
    Attention needed from Danilo Tedeschi and Nicholas Verne

    Tim added 1 comment

    Patchset-level comments

    High level question on this one: I like the move to LanguageTag::GetParentTag, but I feel we've now got two very similar loops in order to get the full chain of tags for both cases. Would it make more sense to keep l10n::GetParents and put the for loop in there instead? Then we don't even need to update the callers and can keep the testing. Alternatively maybe adding a LanguageTag::GetParentTags (plural) for this behavior?

    Tim

    Note sure if you saw this comment from my previous reply.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Danilo Tedeschi
    • Nicholas Verne
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: Id2737ea3a8c819b27648900faf1e44fcddac7085
      Gerrit-Change-Number: 8120546
      Gerrit-PatchSet: 13
      Gerrit-Owner: Danilo Tedeschi <da...@google.com>
      Gerrit-Reviewer: Danilo Tedeschi <da...@google.com>
      Gerrit-Reviewer: Nicholas Verne <nve...@chromium.org>
      Gerrit-Reviewer: Tim <tjud...@chromium.org>
      Gerrit-Attention: Danilo Tedeschi <da...@google.com>
      Gerrit-Attention: Nicholas Verne <nve...@chromium.org>
      Gerrit-Comment-Date: Fri, 24 Jul 2026 23:16:00 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Nicholas Verne (Gerrit)

      unread,
      1:03 AM (3 hours ago) 1:03 AM
      to Danilo Tedeschi, Tim, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, extension...@chromium.org, chromium-a...@chromium.org, jshin...@chromium.org, crost...@chromium.org
      Attention needed from Danilo Tedeschi

      Nicholas Verne voted

      Code-Review+1
      Commit-Queue+2
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Danilo Tedeschi
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Id2737ea3a8c819b27648900faf1e44fcddac7085
        Gerrit-Change-Number: 8120546
        Gerrit-PatchSet: 13
        Gerrit-Owner: Danilo Tedeschi <da...@google.com>
        Gerrit-Reviewer: Danilo Tedeschi <da...@google.com>
        Gerrit-Reviewer: Nicholas Verne <nve...@chromium.org>
        Gerrit-Reviewer: Tim <tjud...@chromium.org>
        Gerrit-Attention: Danilo Tedeschi <da...@google.com>
        Gerrit-Comment-Date: Sat, 25 Jul 2026 05:03:01 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages