ImpressCMS / impresscms

A multilingual, extensible, community oriented CMS developed in PHP
https://www.impresscms.org
Other
27 stars 35 forks source link

CodeClimate is taking issue with tabs #455

Closed fiammybe closed 5 years ago

fiammybe commented 5 years ago

let's adapt the codeclimate settings, because otherwise we will have lots and lots of files to adapt :-)

MekDrop commented 5 years ago

spaces for me would be easier (>99% of my coding work usually written with spaces) but if I remember correctly @skenow likes very much tabs. So, if nothing has changed here probably we should fix codeclimate rules instead for that.

fiammybe commented 5 years ago

@skenow is concerned about the speed loss of more characters in the file. He could have a point. If we want to, we could do some statistical analysis of loading and executing the same files with tabs and spaces, and see if there is a difference.

skenow commented 5 years ago

Let’s just get rid of indentation altogether and end the discussion.

Adding spaces adds no value and increases the possibility of file diffs and adds to file sizes.

On Tue, May 28, 2019 at 5:59 AM David Janssens notifications@github.com wrote:

@skenow https://github.com/skenow is concerned about the speed loss of more characters in the file. He could have a point. If we want to, we could do some statistical analysis of loading and executing the same files with tabs and spaces, and see if there is a difference.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/ImpressCMS/impresscms/issues/455?email_source=notifications&email_token=AASRBM5ZEY7DCQD3MPWBDLDPXUGAZA5CNFSM4HQBHQHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWLYFBY#issuecomment-496468615, or mute the thread https://github.com/notifications/unsubscribe-auth/AASRBM4VACJV42MJVVLJLILPXUGAZANCNFSM4HQBHQHA .

skenow commented 5 years ago

Sorry for the tone of my previous comment. There was a time when we focused heavily on improving performance and reducing overhead and I tallied a lot of hours running comparisons. For me, it wasn't, and still isn't, just a matter of preference. It now seems irrelevant.

My benchmark was to run it on the slowest machine available, with no benefit from any other tool (optimizer or caching), and if it performed well there, it would scale to the enterprise level, running very large and busy sites.

Ultimately, the amount of whitespace in pre-compiled code makes little difference, but does make some, however small. If the server also has an optimizer/accelerator, it makes even less of a difference to the actual site performance. Just being able to run on PHP 7 makes an improvement in perceived performance improvements.

With all the automation available in your IDE and GitHub, does it really matter? What really matters is that what we deliver functions and performs well.

fiammybe commented 5 years ago

Personally, I couldn"'t care less about tabs or spaces. As a matter of fact, what ImpressCMS lacks at the moment is features, not actual or perceived speed. I'll get this check out of codeclimate, and to me, as long as it improves the pace of developments, spaces/tabs isn't a factor I'm considering anymore for PRs.

fiammybe commented 5 years ago

The way I can see it, we already have tabs defined as spacing (looking at the gist file mentioned in the codeclimate configuration file).

MekDrop commented 5 years ago

I think this issue was because I created merge from very old branch (that I forgot for long time that existed). So closing.