Review Request 112567: Move the tab-handling away from DolphinMainWindow (Patch-Series)

[Frank Reininghaus]

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


Thanks for your efforts to refactor the tab-handling code.

Hm, I'm not quite sure what I and possible other reviewers are supposed to say about this review request. I think that a patch that removes crucial functionality should not be pushed to master. There are quite a few people (including myself) who use the master branch for their daily work, and most of them would not be happy if splitting the view and opening tabs suddenly does not work any more, even if it's just a temporary regression.

I like the idea of splitting huge changes into smaller independent commits, but also the other two patches are a bit hard to comment on, considering that I don't know what the final result is supposed to be, and that I haven't worked much with that code in DolphinMainWindow at all.

Maybe your patch series could be committed to a feature branch? That would enable others to look at all patches in context and to test it. I would really appreciate any help from others here. As I said, I don't know that code well, and going through all this and comparing with the old code will probably take *many* hours.

I think it might also help if you could provide a high-level overview of what your patch series does, what new classes there are, etc. And maybe even more importantly, what is the benefit of the change? To be honest, at the moment it looks to me like you are proposing to turn a lot of code (which has worked well for a couple of years) upside down, and I'm not sure if I understand what the purpose of the change is.

Please try to make the review job a bit easier for everyone else. I cannot really spend more than an hour per day on Dolphin on average, and my motivation to spend at the very least a week, possibly more, of the daily "Dolphin time" figuring out all that stuff myself, seeing the bug reports pile up during that time, and getting no work done myself is rather low if I don't see the "big picture" and I don't know what the purpose of the change is.

- Frank Reininghaus


On Sept. 6, 2013, 4:48 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112567/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2013, 4:48 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> Move the tab-handling away from DolphinMainWindow (Patch-Series)
>     
> Removed all code in DolphinMainWindow which is related to tabbed browsing,
> except of some functions/slots which are required for the current UI, like
> openNewTab(), openInNewTab(), ...
>     
> The tab-handling will be implemented again in another patch.
> 
> Split View and Tabbed Browsing will not work anymore with this patch!
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinmainwindow.h 7da5801 
>   dolphin/src/dolphinmainwindow.cpp 4128cdf 
> 
> Diff: http://git.reviewboard.kde.org/r/112567/diff/
> 
> 
> Testing
> -------
> 
> Split View and Tabbed Browsing doesn't work (is ok)
> 
> Everything else works fine for me (Only one view)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130908/d1182726/attachment.htm>