search

sct / League

A simple PHP wrapper for the official League of Legends API
MIT License
10 stars 1 forks source link

Fixed coding standards, compliance to PSR-2 #1

Closed tristanbes closed 10 years ago

sct commented 10 years ago

I am uncomfortable accepting a pull request that is 100% changes that just switches tabs to spaces.

Is there anything else you changed? I can fix this myself with a quick commit. I just want to know what else you may have noticed that isn't compliant without me having to read through your entire commit.

tristanbes commented 10 years ago

Haven't you heard of PSR-2 compliance (http://www.php-fig.org/psr/psr-2/) ? I've fixed equals alignement, brackets for controls structure, I translated tabs into spaces and various other coding standards level.

Why fix it yourself if I already took time to fix it ? Is not that the purpose of github and code collaboration ?

tristanbes commented 10 years ago

PS: You shouldn't be unconfortable accepting ANY valuable PR including ANY improvements or fix.

I can find you hundreds of accepted PR on popular open source frameworks that is based only on filemode changes, space problem or any other CS violations.

sct commented 10 years ago

I am probably just being overly selfish about the contribution amounts on Github. This is a 100% contribution PR for just compliance. I already fixed it myself locally.

I do understand you are just trying to help but this overall was just a mistake I made in configuring the project in Sublime Text. (As well as missing brackets on the one control structure)

I am pushing the changes now. If you still see anything wrong with the project (alignment issues, missing brackets) feel free to submit another PR and I will accept it.

Again, this isn't about me being against PRs. It's just me being against a PR that is 100% contribution when all you did was change tabs to spaces. I hope you understand.

tristanbes commented 10 years ago

You fixed one among other CS problem :-1: . Your code is far away from being PSR-2 valid.

If you're not willing to accept PR because you're afraid to loose some kind of virtual fame, then you should consider setting the repo to read only mode and disabling PR features.

I wanted to use your library in order to make a Symfony2 Bundle, but I won't use it if it's not PSR-2 compliant.

I took time to make the PR, and i won't take more time to explain to you any rules composed by PSR-2.

PS: Having a repository which have contributors is more rewarding than "I made it myself alone".

You closing my PR : giphy

sct commented 10 years ago

I really don't see the differences between the two? Maybe I am making some sort of huge mistake here? That's possible.

I am willing to swallow my pride here and ask you what the major problems are with my compliance.

I will accept your PR and we can go from there. In the end I would rather learn from this than just blow you off.

sct commented 10 years ago

Thank you for taking the time to point these things out. I feel pretty stupid for missing the class declaration bracket on a new line.

I did add new lines at the end of my files and switch to spaces instead of tabs. I also know about brackets for control statements.

tristanbes commented 10 years ago

Thanks. I'll sleep better tonight. ;)