Skip to content

CLDR-17443 Fix person-name problem in Spanish #4954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Sep 3, 2025
Merged

CLDR-17443 Fix person-name problem in Spanish #4954

merged 2 commits into from
Sep 3, 2025

Conversation

macchiati
Copy link
Member

The alt and plain forms should differ by {surname2}, as pointed out in the ticket. (It doesn't matter which, so just following the medium choice). Look at the ticket for details.

CLDR-17443

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

@macchiati
CLDR-17443 Fix person-name problem
4691a00 
The alt and plain forms should differ by {surname2}, as pointed out in the ticket. (It doesn't matter which, so just following the medium choice).
@macchiati macchiati marked this pull request as ready for review August 11, 2025 21:46
richgillam
richgillam previously approved these changes Aug 11, 2025
View reviewed changes
Copy link
Contributor

@richgillam richgillam left a comment

Choose a reason for hiding this comment

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

I just reread the ticket, and it seems to be complaining about a lot of other things in addition to this issue. Does this change cover everything the ticket complains about? If not, are the other complaints legitimate, and does that mean we need to fix them too, keep this ticket open, or file a new ticket?

But yes, this change LOKTM.

@macchiati
Copy link
Member Author

Whoops, neglected to regenerate the test data. Will follow up with commit.

@macchiati
Copy link
Member Author

macchiati commented Aug 12, 2025 •
edited
Loading

There are three items mentioned:

  1. The two namePatterns for es locale for order="sorting" length="long" usage="referring" formality="informal" are the same
    • This was fixed
  2. Using that namePattern with a data record with no surname results a combination of two strings that seems incorrect being " , " using the combining rules in the specification
    • The fix was to produce the desired output
  3. The CLDR test cases generate a result that seems non-compliant with the spec, and possible also incorrect.
    • I don't think the test cases are non-compliant (I regenerated with the fix); I think the problem was with the data. However, I can hive off that part into a separate subtask for us to check whether it is the case, and if so, fix in the spec. (The deadline for data changes is fast approaching, whereas we have more time for spec changes.)

@macchiati macchiati requested a review from richgillam August 12, 2025 00:45
@macchiati macchiati requested a review from srl295 September 2, 2025 17:39
@macchiati
Copy link
Member Author

Adding SRL (I think Rich and Peter are both out)

@macchiati macchiati requested a review from AEApple September 2, 2025 17:58
@AEApple
Copy link
Contributor

AEApple commented Sep 3, 2025

Is there a ticket tracking adding a test for this if they need to be different?

@macchiati macchiati merged commit ffed4aa into main Sep 3, 2025
17 checks passed
@macchiati macchiati deleted the CLDR-17443-macchiati-Spanish-surname2-problem branch September 3, 2025 14:54
@macchiati
Copy link
Member Author

Is there a ticket tracking adding a test for this if they need to be different?

Good idea. I should file a ticket for 49 before we close the ticket

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

@AEApple AEApple AEApple approved these changes

@pedberg-icu pedberg-icu Awaiting requested review from pedberg-icu

@richgillam richgillam Awaiting requested review from richgillam

@srl295 srl295 Awaiting requested review from srl295

Assignees

@macchiati macchiati

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@macchiati @AEApple @richgillam