[Kde-pim] Review Request: Cleanup coding style other the directory kdepim-runtime/maildir/

[Guy Maurel]

> On May 5, 2012, 3:10 p.m., Kevin Krammer wrote:
> > resources/maildir/libmaildir/maildir.cpp, line 536
> > <http://git.reviewboard.kde.org/r/104864/diff/1/?file=62821#file62821line536>
> >
> >     I think
> >     QLatin1Char( '.' )
> >     would be even better

well! 
This was notice by krazy, and I have made here only the changes of "space(s)".
In fact, your proposal is the better one


> On May 5, 2012, 3:10 p.m., Kevin Krammer wrote:
> > resources/maildir/libmaildir/maildir.cpp, line 508
> > <http://git.reviewboard.kde.org/r/104864/diff/1/?file=62821#file62821line508>
> >
> >     (qint64) is not a function call but a C style cast, so not spaces in there.
> >     What you could do instead is
> >     qint64( 8000 )
> >     i.e. calling the qint64 "constructor"

right!
This is the problem of a too simple parser.


> On May 5, 2012, 3:10 p.m., Kevin Krammer wrote:
> > resources/maildir/libmaildir/maildir.cpp, line 622
> > <http://git.reviewboard.kde.org/r/104864/diff/1/?file=62821#file62821line622>
> >
> >     I think you can make those other ifs into else ifs, i.e. flag cannot be Forwarded and Replied, etc

what about the use of
switch ( flag ) {
  case konadi::MessageFlags::Forwarded:
    mailDirFlags << QLatin1String( "P" );
    break;

uso...


> On May 5, 2012, 3:10 p.m., Kevin Krammer wrote:
> > resources/maildir/libmaildir/maildir.cpp, line 785
> > <http://git.reviewboard.kde.org/r/104864/diff/1/?file=62821#file62821line785>
> >
> >     Make that a Q_FOREACH for consistency
> >     and const QString &keys (moving the & towards the variable name)

Is it possible to change *ALL* the foreach?
The change of the position of & is a question that we should discuss, because there more, more other places of such a coding style.
Other meanings?? 


> On May 5, 2012, 3:10 p.m., Kevin Krammer wrote:
> > resources/maildir/maildirresource.cpp, line 745
> > <http://git.reviewboard.kde.org/r/104864/diff/1/?file=62824#file62824line745>
> >
> >     move & towards variable name, i.e.
> >     const Item::Flag &flag

siehe my question above


> On May 5, 2012, 3:10 p.m., Kevin Krammer wrote:
> > resources/maildir/retrieveitemsjob.cpp, line 127
> > <http://git.reviewboard.kde.org/r/104864/diff/1/?file=62825#file62825line127>
> >
> >     } else {

well, the code is correct in this form (I think)
Your wish to enclose the ONLY ONE next line should be discuss in the group and (maybe) applied at more, more positions as
a coding style. My opinion is: I would be better to read and give the garantee of less errors if somebody want to make a
change in the the "else"-part.


- Guy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104864/#review13471
-----------------------------------------------------------


On May 5, 2012, 2:37 p.m., Guy Maurel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104864/
> -----------------------------------------------------------
> 
> (Updated May 5, 2012, 2:37 p.m.)
> 
> 
> Review request for KDEPIM and Kevin Krammer.
> 
> 
> Description
> -------
> 
> Cleanup coding style other the directory kdepim-runtime/maildir/
> Note: NO signature where changed
> 
> Changing all the occurences of,
> from                 to
> ! expression         !expression
> ))                   ) )
> ){                   ) {
> if(                  if (
> foreach(             foreach (
> do{                  do {
> }while               } while
> ("string")           ( "string" ) 
> (expression)         ( expression ) 
> space at the end of a line
> 
> 
> Diffs
> -----
> 
>   resources/maildir/configdialog.cpp 1ca4c79 
>   resources/maildir/libmaildir/keycache.cpp 83641df 
>   resources/maildir/libmaildir/maildir.h 8de40c3 
>   resources/maildir/libmaildir/maildir.cpp e64fb91 
>   resources/maildir/libmaildir/tests/testmaildir.cpp f7ecd73 
>   resources/maildir/maildirresource.h 41f35b5 
>   resources/maildir/maildirresource.cpp e8134fb 
>   resources/maildir/retrieveitemsjob.cpp c87c7b7 
>   resources/maildir/tests/synctest.cpp 2d13287 
> 
> Diff: http://git.reviewboard.kde.org/r/104864/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guy Maurel
> 
>

_______________________________________________
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/