search

jonathangeiger / kohana-jelly

See the link below for the most up-to-date code
https://github.com/creatoro/jelly
MIT License
146 stars 34 forks source link

Coding style: Unwanted whitespace and newline at EOF #100

Closed Lokaltog closed 14 years ago

Lokaltog commented 14 years ago

I've noticed that there's a lot of unwanted/trailing whitespace in Jelly, as well as missing newlines at the end of some files. I consider it good practice to remove trailing whitespace from all files before committing them, and trailing whitespace is also removed from all files in Kohana core.

I use an autocmd in vim to remove all trailing whitespace when saving PHP files, and vim also automatically adds a newline at EOF to all text files. This messes up the commits when I change files in Jelly, like this one.

I know I can just change my configuration to not remove trailing whitespace, but I'd like to know if there's any reason for keeping whitespace in Jelly. If not, I would suggest that someone does some whitespace cleanup (I'd gladly do it) in all Jelly files. :)

banks commented 14 years ago

I would suggest that someone does some whitespace cleanup (I'd gladly do it)

Be our guest! We have no intentions to add trailing whitespace but I guess since different people with different editors work on the code, it is hard to say who or whose editor configurations are doing some of this stuff.

Congrats on posting issue #100!

Lokaltog commented 14 years ago

Thanks! :P

I've cleaned up whitespace in all the files and chmodded classes/jelly/model/core.php to 644 like all the other files: 15c1f6fee414f113aa72309b2babaddcfc3aafbb.

banks commented 14 years ago

chmodded classes/jelly/model/core.php to 644

I wasn't aware that git stored file permissions - if something was funny on your machine it isn't anything to do with the repository. Please correct me if I'm wrong.

Lokaltog commented 14 years ago

Yeah, git stores permissions. If you check out my commit you'll see this:

classes/jelly/model/core.php 100755 → 100644

It was 755 in 28b73ef712aeebcbeb9d2a48368d175011e0acac (proof). :)

banks commented 14 years ago

I stand corrected. Good to know. Thanks. We'll merge this cleanup when one of us gets a chance - I have a crazy couple of weeks coming up and Jon has just had one but hopefully we'll get some more time to work on this code soon...

jonathangeiger commented 14 years ago

Closed by 15c1f6fee414f113aa72309b2babaddcfc3aafbb. Thanks alot!