Closed GoogleCodeExporter closed 9 years ago
I won't take the patch. If I don't have a document describing my coding style
it doesn't mean I don't have one.
1. Indentation: you're using 4 spaces, I use tabs.
2. A lot of mess like unnecessary brackets in if statements.
3. Spaces separating arguments and brackets in function calls.
4. Bugs: there is no such thing as os.exit it's sys.exit. If you haven't
noticed that, it means you haven't even tested it. It's Python after all, you
can't send a python patch to anyone without testing it.
5. shutil.copyfile( path, path + str( time.time() ) + ".bak" ) # I don't need
undocumented changes like that, if you're making a patch it should fix
something or add something, not some random stuff. Generally speaking, that way
of doing backups is unacceptable. Users will end up with tons of backup files
in their config dirs. I agree that using shutil is better than os.system("cp")
though.
So.. fix all that stuff, or I won't accept it.
Original comment by no.smile...@gmail.com
on 29 Nov 2010 at 7:12
Issue 3 has been merged into this issue.
Original comment by no.smile...@gmail.com
on 29 Nov 2010 at 7:13
> I won't take the patch.
Well, you're free to do that. I just wanted to share the enhancement of obkey
with everybody. It now works fine (for me).
> If I don't have a document describing my coding style it doesn't mean I don't
have one.
How am I then supposed to guess your coding style?
> 1.Tabs
> 2.A lot of mess like unnecessary brackets in if statements.
> 3. Spaces separating arguments and brackets in function calls.
It does not seem a very big deal... is it? It is not a so long script so you're
unable to change it accordingly to your tastes.
> 4. Bugs: there is no such thing as os.exit it's sys.exit. If you haven't
noticed that, it means you haven't even tested it. It's Python after all, you
can't send a python patch to anyone without testing it.
The obkey script, with that patch applied, is perfectly working now. Yep, it is
true that sys.exit() is the correct one in that places, but the other
modifications should be okay, as they work for me (that branch of the code is
only reached when everything else, that's why it is not tested).
> So. fix all that stuff, or I won't accept it.
As I said, is up to you, I won't be sending any other file. I was just trying
to help, and I really think that obkey is better now, being able to show errors
properly, inferring the configuration file to use, or accepting a new one as
argument... I'd at least use the showError() function, and surround your code
with try...except. After all, that's simply a matter common sense.
Sorry to have disturbed you.
I wonder anyway if you are really willing to get help.
Original comment by baltasarq
on 30 Nov 2010 at 10:27
> As I said, is up to you, I won't be sending any other file.
Goodbye.
Original comment by no.smile...@gmail.com
on 30 Nov 2010 at 10:54
Original issue reported on code.google.com by
baltasarq
on 29 Nov 2010 at 3:52Attachments: