Merge lp:~linuxdcpp-team/linuxdcpp/0707-core into lp:linuxdcpp

[Steven Sheehy]

[Razzloss (razzloss)]

review: Approve

[Steven Sheehy (steven-sheehy)]

I've found quite a few bugs in my testing of this branch. Do you think it would be better to merge the change as is and fix them later or to try to fix the bugs in the branch then merge?

[Razzloss (razzloss)]

I guess that depends on bugs you've found. Showstoppers and annoying bugs should be fixed before, but anything else could be fixed after the merge. And if you don't intend to fix those yourself a list of the issues found would be nice (I probably have time this week).

On that note couple of things that there is:
  * sounds we're removed during the upgrade (gtk_beeps) as the setting name for notifications changed from simple bool-value to string (I guess that was to allow specific sound files to be played on notifications)
  * Parent row for downloads isn't always updated and shows an incorrect state. (sometimes when the last download fails or completes parent state isn't updated)

But I'd say with these kind of bugs it would be ok to merge and fix later.

--RZ

[Steven Sheehy (steven-sheehy)]

Some of the issues are just questions I have about of some of the new functionality. Here's the list:

1) Download cancelled but segmented downloads still shows up in finished download
2) Shouldn't files show up in finished downloads only once they are 100% complete?
3) What is the point of the group by users in finished *loads? Seems unnecessary and doesn't look aesthetically pleasing with two rows of tabs.
4) When users first connect they show up as a parent row, then once they connect they move to be a child
5) Download filelist from a user who just finished a segment for another file results in filelist download showing up as a child of the non-filelist download
6) Incorrect time left calculated sometimes (ex. 0:-3:-31) (I saw it with a download with only one source, not sure if this is relevant)
7) Instead of copyListValue_gui and copyTreeValue_gui, would it be better just to have copyValue_gui and let overloading do its work?
8) Should we be using namespace dcpp or use the namespace specifier everywhere to make it clear where the object comes from?

If you make any changes, please make them to the 0707 branch as I am in the process of merging to trunk.

[qwertitis (qwertitis-deactivatedaccount)]

Regarding point number 3 (the group by user/file tabs in finished transfers)

Aside from the necessity of the grouping functionality, those two tabs are terrible UI design. Notebooks should change contents, not just view the same information with a different view mode.

Surely one can do this without the tabs. In file managers, they do similar view-change functionality (detailed list, icon view, thumbnail view...) with radio buttons in a toolbar.

Just my thoughts on the subject.

[Razzloss (razzloss)]

1) True, if it had some segments finished before cancelling. You think that the already finished segments should be cleared from finished dls when cancelling?
2) Probably, but I'm not sure if it can be reliably done with 0.707. Next versions had isCompleted() or something. You can grab the complete size of a file from QueueManager if it still has it and compare that to the transferred amount, but no idea how QueueManager behaves when the file is downloaded in single segment and the file is already downloaded when 1st segment completes.
3) Aesthetically pleasing GTK-app.. now that would be something ;). No I don't really see the point either, but this is the way it's done in windows clients and the core supported it without trouble.
  * If anyone wants to make the lists cleaner (ie. just completed dls, without the 2 different groupings, make it work as it's supposed to, etc) go ahead. I have no problems with that, but I've already wasted enough hours trying to get that thing to do that so I won't bother again. (had few solutions but those were either too fugly or too slow, so I gave up and copied the dc++ implementation)
  * Individ: meh, not my design. Just copied the UI from dc++ without thinking that much.

4) Where they should be put when connecting? Add parent row for Connecting users and put them all there?
5) Ok, that's a bug and easily fixed. Should the filelist downloads be parent rows (without children of course) or child of a parent with just that one filelist dl, or should all the filelist dls be grouped under one parent?
6) There's a report of that already. I didn't manage to reproduce it with the instructions in the report, but do you have any clues on how to reproduce it?
7) Sounds sane, copyListValue isn't used anywhere, AFAIR, I added it only because it might be useful later..
8) Up for debate. I only added those using -declarations to test if 705 would compile easily. I didn't intend to end up porting the damn thing completely.

--RZ

[Steven Sheehy (steven-sheehy)]

1) Yes, or more accurately, I think they should never be added to begin with (see #2).
2) That's fine, we'll get 0.707 merged for now and figure out how to accomplish this later. After we get it merged, I suggest that we go ahead and merge the latest stable DC++ core before we make a release, which will allow us to utilize the isCompleted() that you mentioned.
3) I may try to take a look at cleaning this up after we get it merged.
4) I don't think a connecting parent row would look good. I think they should be a child row just like downloading segments. No idea if that is possible technically, just my point of view.
5) Definitely not all grouped under one file list parent. The other two are up for debate. I think we should probably keep the behavior consistent with regular downloads whichever we choose. Which is another issue that needs to be discussed: Should there be a child if there's only one segment?
6) Not really, sorry.
7) Ok, I'll rename them then.
8) For now I'll move using namespace dcpp out of the headers, at least. Eventually I'd like to see us remove them out of the .cpp as well, but this can wait until later. In the future I'd like to see us have our own namespace as well.