[Kde-pim] Review Request 123495: Parse Date: field from ticketmaster emails

Sandro Knauß knauss at kolabsys.com
Mon Apr 27 09:09:23 BST 2015



> On April 26, 2015, 12:26 nachm., Sandro Knauß wrote:
> > kmime/kmime_header_parsing.cpp, line 2033
> > <https://git.reviewboard.kde.org/r/123495/diff/1/?file=363044#file363044line2033>
> >
> >     Your current test do not text the new branch of the second part after ":" (it is always 0, what is boring :)
> >     
> >     So please add a test for:
> >     
> >     "Fri 24 Apr 2015 10:39:15 +02:23"
> >     
> >     also make sure that it fails with incorrect values aka:
> >     
> >     Fri 24 Apr 2015 10:39:15 +02:23:45
> >     Fri 24 Apr 2015 10:39:15 +02asdf
> >     Fri 24 Apr 2015 10:39:15 +02:af
> >     Your current test do not text the new branch of the second part after ":" (line 2033 ff).
> >     
> >     So please add a test for something like:
> >     
> >     "Fri 24 Apr 2015 10:39:15 +02:23"
> >     
> >     also make sure that it fails with incorrect values aka:
> >     
> >     Fri 24 Apr 2015 10:39:15 +02:23:45
> >     Fri 24 Apr 2015 10:39:15 +02asdf
> >     Fri 24 Apr 2015 10:39:15 +02:af
> >     Fri 24 Apr 2015 10:39:15 +in:af
> 
> Albert Astals Cid wrote:
>     There is a problem with
>     Fri 24 Apr 2015 10:39:15 +02:23:45
>     being tought as correct, as well as the old code also detects
>     Fri 24 Apr 2015 10:39:15 +0223AADS
>     as correct, because parseDigits("0223AADS") will return 4 that is what the code checks for.
>     
>     Honestly I'd prefer to live this "negative checking" out of this patch, since the patch is to make the email from ticketmaster work, making other "wrong" cases that did not fail before fail is imho out of scope.

Just make sure, that every other format than hh:mm behaves the same than before the patch. And at least one test date should trigger line 2034 (return false).


- Sandro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123495/#review79528
-----------------------------------------------------------


On April 25, 2015, 1:53 nachm., Albert Astals Cid wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123495/
> -----------------------------------------------------------
> 
> (Updated April 25, 2015, 1:53 nachm.)
> 
> 
> Review request for KDEPIM-Libraries, Sandro Knauß, Thomas McGuire, and Allen Winter.
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> ticketmaster emails have the timezone with a : in the middle
> 
> This is probably mal formed but parsing them is not hard and will make me not panic thinking i didn't receive the email for the tickets i just bought.
> 
> 
> Diffs
> -----
> 
>   kmime/kmime_header_parsing.cpp 1043738 
>   kmime/tests/auto/CMakeLists.txt 7fe677e 
>   kmime/tests/auto/parsedatetimetest.h PRE-CREATION 
>   kmime/tests/auto/parsedatetimetest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/123495/diff/
> 
> 
> Testing
> -------
> 
> I added a small autotest \o/
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


More information about the kde-pim mailing list