D5627: Remove KDELibs4Support from KSudoku

[Johan Ouwerkerk]

ouwerkerk requested changes to this revision.
ouwerkerk added a comment.
This revision now requires changes to proceed.


  I've added a few more comments. Before landing this change please review the one on copyright in particular, as I'm not certain the way a few copyright statements were consolidated is sound (or rather that the implications are what was intended).

INLINE COMMENTS

> ksudoku.cpp:5
>   *   Copyright 2006-2008 Johannes Bergmeier <johannes.bergmeier at gmx.net>   *
> - *   Copyright 2012      Ian Wadham <iandw.au at gmail.com>                   *
> - *   Copyright 2015      Ian Wadham <iandw.au at gmail.com>                   *
> + *   Copyright 2012-2015 Ian Wadham <iandw.au at gmail.com>                   *
>   *                                                                         *

This change may not be sound:

As I understand it:

- Copyright 2012 means copyright on the work for the year 2012
- Copyright 2012, 2015 means copyright on the work for the years 2012 and 2015.
- Copyright 2012-2015 means copyright on the work for the years 2012 thru 2015 (inclusive), i.e. 2012, 2013, 2014 and 2015.

Since the previous version did not claim copyright over 2013-2014, perhaps we should either leave both lines as is or maybe consolidate using `Copyright 2012, 2015` instead?

> stikonas wrote in serializer.cpp:402
> Maybe do refactoring in a separate patch? This needs to fix calls to these functions too then... But yeah, that sounds good.

Agreed.

> serializer.cpp:472
> +	}
> +	success = true;
> +

Maybe I'm losing track here, but I think `success` is no longer relevant? Because the code now short-circuits by returning early, we only get here on success. The following `if()` check has therefore become redundant as well.

REPOSITORY
  R417 KSudoku

REVISION DETAIL
  https://phabricator.kde.org/D5627

To: stikonas, #kde_games, ltoscano, ouwerkerk
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20170428/56eca84f/attachment.html>