Rebuilding Format class #3149
Rebuilding Format class #3149
Conversation
creating new class Format to handle getFormatter method
new exception methods forInvalidFormatter & forInvalidMime using in getFormatter
adding invalidFormatter & invalidMime translation using in FormatException
adding some changes to current format codes to be compatible with \CodeIgniter\Format
adding some changes to current format codes to be compatible with \CodeIgniter\Format
adding format as instance of factory for Formatters
| * | ||
| * @return \CodeIgniter\Format\FormatterInterface | ||
| */ | ||
| public function getFormatter(string $mime) |
There was a problem hiding this comment.
We can't remove this. It would be a BC change.
This should use Services::format() under the hood and have a phpdoc comment that points out this method is deprecated since we have a new Format class now.
There was a problem hiding this comment.
I didn't remove it completely i just relocated it in system/Format/Format.php under CodeIgniter\Format namespace and now all Format instances runs throw \Config\Services::format()
about tests I actually don't know how to do it on the right way, may i get help or something
There was a problem hiding this comment.
Yes, I'm aware that you moved it to the Format class, but it doesn't change the fact that some people might use this method already in their code (just like we do in some of our tests). If we remove this method from here it will break their code - it has to stay.
There was a problem hiding this comment.
I got your point, but i think we can solve this problem by hinting new changes in next version changelog or something like that, any ideas?
There was a problem hiding this comment.
No, sorry. It has to stay - it was a public method. Removing it will be a breaking compatibility change.
adding some changes to current format codes to be compatible with \CodeIgniter\Format removing negotiate() methord 3rd argument its false by default removing useless property $this->formatter
|
Travis faild i will do changes at night |
|
If you look at Travis build, you will see some errors. Please change the code in these tests so they use new Speaking about tests for Of course, test it locally first: |
but shall I use (! empty($this->config) or just ! instanceof as I did
revert some changes and improve code style
One line code
replace with new format() service
keeping configuration methods away of hands