fix(person): avoid repeated double last name in some locales #1818
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## next #1818 +/- ##
==========================================
+ Coverage 99.62% 99.63% +0.01%
==========================================
Files 2346 2346
Lines 235646 235650 +4
Branches 1135 1145 +10
==========================================
+ Hits 234754 234793 +39
+ Misses 870 835 -35
Partials 22 22
|
There was a problem hiding this comment.
I think this PR tries to fix a problem, without taking the cause of the problem into consideration.
There are two problems here:
- The patterns seem to express an additional context to the last names that they are unable to express: The person doesn't have a single last name, but two.
- Some of the patterns seem to modify the exontext of last name to express complex last names such as combined/joined last names.
IMO the real solution to this problem is removing the repeated lastNames from the pattern and actually pass the entire last name to the method directly. (Where would the other last name come from anyway in legal documents? "John" + "Smith" ⇏ "John Smith Foobar")
If there is a locale that actually has two separate last names, then it has to use a different/not yet existing method or parameter set for it. If a locale has some kind of double last name, then it should have them in the locale data itself. Similar to thetussenvoegsel (#1778).
What do you think?
| @@ -8,11 +8,11 @@ export default [ | |||
| weight: 1, | |||
| }, | |||
| { | |||
| value: '{{person.firstName}} {{person.firstName}} {{person.lastName}}', | |||
| value: '{{person.firstName}} {{person.firstName}} {{person.lastName2}}', | |||
There was a problem hiding this comment.
This pattern only has a single last name.
|
I don't think it's quite like tuessenvoegels because there's no "common" double or hyphenated names. In most cultures it's just your mothers and fathers surnames concatenated in some way. So if there are N simple last names there are N^2 compound last names. I guess we could add an optional last_name_pattern. Then if provided that would affect both lastName() and fullName() |
It doesn't has to, just have them in our sample set.
That might be a alternative worth exploring. (We originally planned to remove all optional data access, so we could throw a useful error message instead of returning undefined, but that is something we can keep in mind when implementing this.) |
i tried out this strategy in #1819 |
|
Team Decision Closing in favor of #1819. |
fix #1817
Since faker.mustache will always replace the same placeholder with the same name, tried introducing lastName2
If you have any better ideas for this let me know.
Note this affects #691 (the hyphenated patterns discussed there are already used in some locales)