search

szarroug3 / X-Ray_Calibre_Plugin

X-Ray Creator plugin for Calibre
http://www.mobileread.com/forums/showthread.php?t=273189
GNU General Public License v3.0
57 stars 12 forks source link

Dev/pylint fixes #80

Closed szarroug3 closed 8 years ago

szarroug3 commented 8 years ago

Made a bunch of pylint fixes

szarroug3 commented 8 years ago

@stoduk Do you wanna take a look at this before I merge? It's quite a big change.

stoduk commented 8 years ago

I can take a look tomorrow, if you are happy to wait until then.

On 2 November 2016 at 15:31, Samreen Zarroug notifications@github.com wrote:

Do you wanna take a look at this before I merge? It's quite a big change.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/szarroug3/X-Ray_Calibre_Plugin/pull/80#issuecomment-257900749, or mute the thread https://github.com/notifications/unsubscribe-auth/AFCvImhCB1Lpj5PlcT9sq1N0Cteu_KmLks5q6KzEgaJpZM4KnYQt .

szarroug3 commented 8 years ago

That's fine. I'll probably add stuff by then

On Wed, Nov 2, 2016 at 12:21 PM Anthony Toole notifications@github.com wrote:

I can take a look tomorrow, if you are happy to wait until then.

On 2 November 2016 at 15:31, Samreen Zarroug notifications@github.com wrote:

Do you wanna take a look at this before I merge? It's quite a big change.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/szarroug3/X-Ray_Calibre_Plugin/pull/80#issuecomment-257900749 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AFCvImhCB1Lpj5PlcT9sq1N0Cteu_KmLks5q6KzEgaJpZM4KnYQt

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/szarroug3/X-Ray_Calibre_Plugin/pull/80#issuecomment-257935416, or mute the thread https://github.com/notifications/unsubscribe-auth/AIEjUXHTndio9XzgrMKTP87NsuOo3UMvks5q6MaWgaJpZM4KnYQt .

szarroug3 commented 8 years ago

@stoduk I changed things according to your review. Want to take another look?

szarroug3 commented 8 years ago

@stoduk moved the templates to separate files.. i didn't need to put the encoding in the json files.. wanna try it on your end to see if it messes anything up without them?

szarroug3 commented 8 years ago

sorry, got caught up and added more stuff lol

szarroug3 commented 8 years ago

@stoduk did you want to look at the latest? if not, I can just merge

stoduk commented 8 years ago

@szarroug3 - sorry, missed your comment a week ago. What were you wanting me to try out? I can't see what would care if the JSON files have an explicit encoding specified or not - are you thinking of anything in particular? As long as the .py files are UTF8-free then pylint will be happy.

Other than that, I think you are good to merge it as it.

szarroug3 commented 8 years ago

@stoduk no problem. I tried it with the codec in the json file and it didn't work.. it gave me this error: ValueError: No JSON object could be decoded

szarroug3 commented 8 years ago

@stoduk I'm going to merge this.. I still have more changes to make anyway so I can put any other changes you want me to make in the next review :)