D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

Thu Oct 18 14:41:51 BST 2018

kossebau marked 2 inline comments as done.
kossebau added a comment.

  In D8708#344872 <https://phabricator.kde.org/D8708#344872>, @dhaumann wrote:

  > Feel free to push this for 5.52 (you'd have to adapt the @since 5.53 again).

  Thanks for review again :)

  Would prefer waiting for 5.53, for some full possible period of testing by people running from master, which should be at least > number of people geting and trying cross-project/repo patches :)

  > I only have minor comments that should not hold back this patch. One thing that pops into my eyes is that there are still some "todo" comments, which even talk about cornercases / bugs. Since I think you know what you are doing, I leave it up to you to decide wither it's ok as is or whether there need to be more fixes.

  Yes, the TODOs left are about non-critical things, like issues already in the old code (those about rendering group delimiters at the lines on the view top/bottom borders), old magic numbers which could see some reasoning, or some check which is practically not needed, but might be expected by some looking at patterns. Left as TODO remarks for my older self or someone else, in case they work on this, so the rough edges are documented and not silently in the code.

  > In any case, it was never my (and I think I can also safely say) nor any other's intention to hold this patch back. So yes, pinging more often is ok imo. Luckily, you are not a first-time contributor (who would be very much discouraged by our review delays), so I am sure / hope we'll see more patches from you in the future as well! :-)

  Okay, white card for more poking about reviews happily received :)
  Some other reason might also have been that I left the ""WIP" too long in the title, which might also have sent a wrong signal, when this was rather RFC on the working prototype/pre-production sample.

INLINE COMMENTS

> dhaumann wrote in kateannotationitemdelegate.h:20
> All keywords starting with __ are reserved, I suggest to remove __ at the beginning and at the end.

Had those to be in line with other headers in that dir. Will do a separate patch to change that for the existing headers then.

REPOSITORY
  R39 KTextEditor

BRANCH
  addAnnotationItemDelegate

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

To: kossebau, #kate, #kdevelop, dhaumann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, sars
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20181018/1d826733/attachment.html>