Create a LanguageTagStaticConverter::KnownLanguageTagFromString [chromium/src : main]

0 views
Skip to first unread message

Greg Thompson (Gerrit)

unread,
Jul 7, 2026, 7:00:43 AM (yesterday) Jul 7
to Danilo Tedeschi, chromium...@chromium.org, jshin...@chromium.org
Attention needed from Danilo Tedeschi

Greg Thompson added 4 comments

Commit Message
Line 12, Patchset 3 (Latest):This is using the IsKnown* functions to decide whether the input forms
Greg Thompson . unresolved

nit: "whether or not" here instead of "or not" at the end of the sentence.

Line 12, Patchset 3 (Latest):This is using the IsKnown* functions to decide whether the input forms
Greg Thompson . unresolved

nit: "uses"

Line 13, Patchset 3 (Latest):an known language tag or not. This allows users to create tags at
Greg Thompson . unresolved

nit: "a"

File base/i18n/tag_converters.h
Line 71, Patchset 3 (Latest):class LanguageTagStaticConverter {
Greg Thompson . unresolved

[Do not use a class simply to group static members.](https://google.github.io/styleguide/cppguide.html#Nonmember,_Static_Member,_and_Global_Functions) can `KnownLanguageTagFromString` just be a function at namespace scope? or can the body of that function be moved into `GetKnownLanguageTag`?

Open in Gerrit

Related details

Attention is currently required from:
  • Danilo Tedeschi
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement 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: I15b29eca0ea582864852a6e84db55824145d2485
Gerrit-Change-Number: 8049020
Gerrit-PatchSet: 3
Gerrit-Owner: Danilo Tedeschi <da...@google.com>
Gerrit-Reviewer: Greg Thompson <g...@chromium.org>
Gerrit-Attention: Danilo Tedeschi <da...@google.com>
Gerrit-Comment-Date: Tue, 07 Jul 2026 11:00:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Danilo Tedeschi (Gerrit)

unread,
Jul 7, 2026, 1:39:02 PM (20 hours ago) Jul 7
to Greg Thompson, chromium...@chromium.org, jshin...@chromium.org
Attention needed from Greg Thompson

Danilo Tedeschi added 4 comments

Commit Message
Line 12, Patchset 3:This is using the IsKnown* functions to decide whether the input forms
Greg Thompson . resolved

nit: "whether or not" here instead of "or not" at the end of the sentence.

Danilo Tedeschi

Done

Line 12, Patchset 3:This is using the IsKnown* functions to decide whether the input forms
Greg Thompson . resolved

nit: "uses"

Danilo Tedeschi

Done

Line 13, Patchset 3:an known language tag or not. This allows users to create tags at
Greg Thompson . resolved

nit: "a"

Danilo Tedeschi

Done

File base/i18n/tag_converters.h
Line 71, Patchset 3:class LanguageTagStaticConverter {
Greg Thompson . resolved

[Do not use a class simply to group static members.](https://google.github.io/styleguide/cppguide.html#Nonmember,_Static_Member,_and_Global_Functions) can `KnownLanguageTagFromString` just be a function at namespace scope? or can the body of that function be moved into `GetKnownLanguageTag`?

Danilo Tedeschi

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Greg Thompson
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement 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: I15b29eca0ea582864852a6e84db55824145d2485
    Gerrit-Change-Number: 8049020
    Gerrit-PatchSet: 8
    Gerrit-Owner: Danilo Tedeschi <da...@google.com>
    Gerrit-Reviewer: Greg Thompson <g...@chromium.org>
    Gerrit-Attention: Greg Thompson <g...@chromium.org>
    Gerrit-Comment-Date: Tue, 07 Jul 2026 17:38:52 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Greg Thompson <g...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Greg Thompson (Gerrit)

    unread,
    3:03 AM (7 hours ago) 3:03 AM
    to Danilo Tedeschi, Chromium LUCI CQ, chromium...@chromium.org, jshin...@chromium.org
    Attention needed from Danilo Tedeschi

    Greg Thompson voted and added 2 comments

    Votes added by Greg Thompson

    Code-Review+1

    2 comments

    File base/i18n/language_tag_unittest.cc
    Line 647, Patchset 8 (Latest):namespace {
    Greg Thompson . unresolved

    nit: blank lines before/after this

    Line 649, Patchset 8 (Latest): constexpr auto kTestData = std::to_array<LanguageTestData>({
    Greg Thompson . unresolved

    nit: `static constexpr`?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Danilo Tedeschi
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement 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: I15b29eca0ea582864852a6e84db55824145d2485
      Gerrit-Change-Number: 8049020
      Gerrit-PatchSet: 8
      Gerrit-Owner: Danilo Tedeschi <da...@google.com>
      Gerrit-Reviewer: Danilo Tedeschi <da...@google.com>
      Gerrit-Reviewer: Greg Thompson <g...@chromium.org>
      Gerrit-Attention: Danilo Tedeschi <da...@google.com>
      Gerrit-Comment-Date: Wed, 08 Jul 2026 07:02:50 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages