Don't fail validation because of datetime-formatting messages. LOC-93 #35
Conversation
|
@sarina A fix! |
|
|
||
| # These should not raise a validation error, because LONG_DATE_FORMAT and | ||
| # friends are special. | ||
| msgid "LONG_DATE_FORMAT" |
There was a problem hiding this comment.
Where do these names come from? How do we know they'll always end in _FORMAT?
There was a problem hiding this comment.
This is a low-tech way to identify these names. I guess I could just make a list instead?
There was a problem hiding this comment.
Hmm, I'd rather keep it like this, in case there are other FORMATs later.
There was a problem hiding this comment.
I guess what I'm asking is, this PR is whitelisting specific strings for
edx-platform, which seems wrong. Wouldn't it be better to allow a variable
whitelist somewhere in your configuration, so that you can quickly extend
the whitelist for specific things in whatever repo the tool is running on?
As we move to IDAs this toolset is being used more generically across more
repos.
On Thu, Dec 17, 2015 at 11:33 AM, Ned Batchelder notifications@github.com
wrote:
In tests/data/validation_problems.po
#35 (comment):@@ -49,3 +49,17 @@ msgstr[1] "2. Estas {num} objectos"
msgid_plural "3. There are {num} things"
msgstr[0] "3. Estas num objectos"
msgstr[1] "3. Estas {num} objectos"
+# These should not raise a validation error, because LONG_DATE_FORMAT and
+# friends are special.
+msgid "LONG_DATE_FORMAT"This is a low-tech way to identify these names. I guess I could just make
a list instead?—
Reply to this email directly or view it on GitHub
https://github.com/edx/i18n-tools/pull/35/files#r47927091.
There was a problem hiding this comment.
For example, there was a time there was a string in edx-platform that was
causing issues because it was an instructor facing string explaining how to
use json properly. So, it would get erroneously validated all the time. I
would have loved a generic way to quickly whitelist that specific string.
On Thu, Dec 17, 2015 at 11:38 AM, Sarina Canelake sarina@edx.org wrote:
I guess what I'm asking is, this PR is whitelisting specific strings for
edx-platform, which seems wrong. Wouldn't it be better to allow a variable
whitelist somewhere in your configuration, so that you can quickly extend
the whitelist for specific things in whatever repo the tool is running on?
As we move to IDAs this toolset is being used more generically across more
repos.On Thu, Dec 17, 2015 at 11:33 AM, Ned Batchelder <notifications@github.com
wrote:
In tests/data/validation_problems.po
#35 (comment):@@ -49,3 +49,17 @@ msgstr[1] "2. Estas {num} objectos"
msgid_plural "3. There are {num} things"
msgstr[0] "3. Estas num objectos"
msgstr[1] "3. Estas {num} objectos"
+# These should not raise a validation error, because LONG_DATE_FORMAT and
+# friends are special.
+msgid "LONG_DATE_FORMAT"This is a low-tech way to identify these names. I guess I could just make
a list instead?—
Reply to this email directly or view it on GitHub
https://github.com/edx/i18n-tools/pull/35/files#r47927091.
There was a problem hiding this comment.
I understand. BTW: the json string is still there, and i'm still thinking about how best to handle that one.
LONG_DATE_FORMAT is a Django string, but I take your point.
There was a problem hiding this comment.
Ahhhhhh OK that's definitely what I was looking for. Since LONG_DATE_FORMAT is a Django string, I'm 👍 on this.
|
how does tests/data/validation_problems.po related to the test files? just adding the data guarentees the tests are run? also why hasn't travis run? |
|
The test I added in the previous PR makes an assertion about all the problems found in validation_problems.po. I've added new strings which should not cause problems, so the test itself doesn't have to change. |
Don't fail validation because of datetime-formatting messages. LOC-93
No description provided.