Implemented setLocalizedDateFormatFromTemplate #671
Conversation
Fix locale passed to the dateFormat(fromTemplate:options:locale:) method to be in line with the documentation
|
I've change a mistake in which locale use in the implementation and add a simple test |
|
|
||
| let f = DateFormatter() | ||
| f.locale = locale | ||
| f.setLocalizedDateFormatFromTemplate(template) |
There was a problem hiding this comment.
What's the benefit of calling f.setLocalizedDateFormatFromTemplate(template) over doing
f.dateFormat = DateFormatter.dateFormat(fromTemplate: dateFormatTemplate, options: 0, locale: locale)?
There was a problem hiding this comment.
Well, here I have to test that the setLocalizedDataFormatFromTemplate will generate the same format as DateFormatter.dateFormat(fromTemplate: dateFormatTemplate, options: 0, locale: locale) if I call directly the latter how can I test it?
There was a problem hiding this comment.
That's not what I meant - though I can see how the placement of this comment made my intent misleading.
I actually meant this as a comment about the entire pull request. I don't really see the added benefit of adding a method like setLocalizedDateFormatFromTemplate() especially when you can just call f.dateFormat = DateFormatter.dateFormat(fromTemplate: dateFormatTemplate, options: 0, locale: locale).
There was a problem hiding this comment.
Oh, right, my bad :)
Maybe you have to ask this question to the Foundation guys inside Cupertino, I merely implement it for removing the NSUnimplemented() call, is a new API added in iOS 10 or 9 if I'm not mistaken
There was a problem hiding this comment.
I am one of the Foundation "guys" 😉. Looking at the implementation of the obj-c version of this method, the method essentially is meant to be purely a convenience so what you've implemented here is fine.
There was a problem hiding this comment.
I will write here because it seems the correct place, I'm sorry for all the people who are working on Foundation and on every other project around the world to have used the term "guys" for reference the group working on Foundation. There are many words to indicate a group of people (like, you know "people") and I'm very sorry to have choose and write the less inclusive of all, I would like to apologise to every person who have read my comment. Please forgive me, and keep up with your amazing work.
|
@swift-ci please test and merge |
I've implemented the missing method in DateFormatter.
I've only two questions:
DateFormatterclass, it is better using the newtype(of:)?dateFormat(fromTemplate:opts:locale:)to get inspiration any guidance on how to add for both the class and the instance method?