-
Notifications
You must be signed in to change notification settings - Fork 778
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
1 changed file
with
17 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f07ca1b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,@jkozera, I am willing to apply to be a co-maintainer!
I am looking forward to your reply.
f07ca1b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I've just made you a collaborator so you can merge pull requests. Let me know if you have any questions.
About your #243, it looks like a good change, but to be honest I'm too confused by the number of various cases to handle to review it completely, but if you think your change is still useful, feel free to merge it yourself.
By various cases I mean:
reply->error() != QNetworkReply::NoError
? It will still show an error message ("Failed retrievinglist of docsets"), won't it?
availMetadata.clear();
called and another message box created in case of some weird failure:!docset[1].startsWith("http")
- would it ever happen? I know I've added this check myself in 8a66b33 but honestly I don't remember why. Maybe it should be removed.clear()
ing the list can make it empty even if the download from kapeli.com succeeded, so it looks wrong, so even if this case ever happens, I wouldn't doclear()
there, and wouldn't show the error message anyway unless both downloads fail.So basically your change is good, but seems like it could be incomplete and not fix all the issues - hence I was hesitating if I should merge it.