-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[WIP] Add full stop at end of validator messages (fixes #2966) #2984
[WIP] Add full stop at end of validator messages (fixes #2966) #2984
Conversation
Hold it a second, missed a few validation messages. |
You'll also need to update the keys in all the various Thanks in advance, though! |
@weierophinney Will do! |
@weierophinney While going through the files, I've come across a number of occurrences of |
@weierophinney Translation keys have been updated. |
I think that this is BC Break since some developers could have manually added the stop in his own code |
I don't mind leaving this for 3.0, but I think there are a few more issues where missing full stops in exception messages are mentioned as well. Plus, the longer the full stops aren't there, the more developers might have put them there in their forms, which means there could be even more pressure against fixing this within the framework's code. I think this should be standardized throughout the framework. @weierophinney What are your experiences regarding exception and validation message standards? |
Trying to find supporting documents. What's this? |
…s done throughout the project with message templates
@localheinz For exception messages, the message should not have a full stop; the reason has to do with the fact that the message becomes part of the exception message reported by PHP (e.g., "Uncaught exception 'Exception' with message 'foo bar' in /some/file/name:19"). As for validation messages, I support the idea of them having full stops. However... I'm worried about what happens for end-users -- will this break their existing code (e.g., if they've defined custom messages already)? Can you test this or ask others to test, please? If there are no issues, I'll merge. If there are, we need to discuss on the ML. |
@weierophinney Will ask on IRC, later. |
As @RalfEggert pointed out on the mailing list, there are a lot of keys contained in the translation files that are not used anymore.
|
@localheinz Based on the ML thread, it sounds like making the changes is a go, with the following approach:
|
@weierophinney I will do, but won't make it this or next week. Thanks! |
@localheinz I cleared the 2.1.0 milestone from this, as it looks unlikely you'll get to it this week. If you do, and we merge, I'll re-add the milestone. |
👍 Too busy at the moment. Will do, though. |
@localheinz Considering the last activity was over a month ago, before 2.1 was released, I'm thinking I should close this, and have you open a new PR against master when you're ready to continue on this. We can then tag the appropriate milestone at that time. |
I also added a few
,
after the last item in the initialization of the validation message templates.