danielfett / advancedcaching

Advanced Geocaching Tool for Linux
http://www.danielfett.de/internet-und-opensource,software,agtl
31 stars 13 forks source link

Various fixes #155

Open ziima opened 11 years ago

ziima commented 11 years ago

Fix several bugs:

ziima commented 11 years ago

I have added two other commits.

First one continues with download if there is error in cache page parsing. Second adds and refactors cache types. It also contains some additional refactoring which should be finished.

ziima commented 11 years ago

Cleaned up the rest of imports. It caused some import bug on N900.

danielfett commented 11 years ago

After all these comments on single commits or lines, I'd first like to thank you for the patches. I will try to use as much as possible. However, it would be easier if the refactoring things would be separated from the changes in functionality.

ziima commented 11 years ago

Ad 1) The download of overview is quite a big method and bit confusing, it should be worth splitting it into several ones. If you think that these flags are the same or similar enough, we should merge them. Ad 2) That is bad :( But the parsing of date format is preference dependent as well, is it not? I checked the HTML, but I did not found no way to differentiate between disabled and archived. Only language-independent way to resolve this from cache page is to download all (!) the logs. Ad 3) I applied this patch to my N900 and it looks good so far :) Ad 4) I my experience, using namespace is fairly common in well behaved python 2 libraries. You might encounter serious problems when you do not differentiate between your imports and import from other libraries. Especially for common names like utils and gui. You might also run into troubles with modules copied from other sources, like threadpool and pyfo. You can check the projects I poked my nose in :) It would be also nice to add a shell script(s) that starts AGTL, so sys.path is not changed on the run. But I did not know how you will react to my refactoring, so I did not do it. Ad 5) Yes, I am sorry for that. At first I just wanted to fix the bugs I found, but I fell into it.

ziima commented 11 years ago

I do not see an changes in devel branch, have you pushed it?