Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Codestyle #34

Open
mablae opened this issue Sep 19, 2016 · 11 comments
Open

Fix Codestyle #34

mablae opened this issue Sep 19, 2016 · 11 comments

Comments

@mablae
Copy link

mablae commented Sep 19, 2016

At least convert everything to leading spaces instead of these "tabs mixins" to start with.

@ejegg
Copy link

ejegg commented Aug 10, 2017

Fixed by #60

@ejegg
Copy link

ejegg commented Aug 10, 2017

This issue is actually pretty important if Amazon wants to benefit from community contributions.

Many of us use IDEs which reformat code to keep indentation consistent when we save a file, or when code is moved from one place to another.
If we want to contribute code to this repository, we have to circumvent this behavior to preserve the inconsistent indentation, or our pull request will end up with a lot of irrelevant changes in the diff (making it more daunting to review).

@bmeynell
Copy link

bmeynell commented Aug 10, 2017

Fix Codestyle

Does this mean following http://www.php-fig.org/psr/psr-1/ and http://www.php-fig.org/psr/psr-2/?

This issue is actually pretty important if Amazon wants to benefit from community contributions.

Agree!

@mglaman
Copy link

mglaman commented Aug 10, 2017

Code style should be PSR-2 for libraries, correct?

@bmeynell
Copy link

@mglaman - Yeah, and is actually a superset of PSR-1.

The style rules herein are derived from commonalities among the various member projects. When various authors collaborate across multiple projects, it helps to have one set of guidelines to be used among all those projects. Thus, the benefit of this guide is not in the rules themselves, but in the sharing of those rules.

@ejegg
Copy link

ejegg commented Aug 10, 2017

Ooh, I was just trying to make the code internally consistent without changing too much. I can make a new PR reformatting it to fit PSR-2.

ejegg added a commit to ejegg/login-and-pay-with-amazon-sdk-php that referenced this issue Aug 10, 2017
Apparently it's the standard for libraries

Fixes amzn#34
@ejegg
Copy link

ejegg commented Sep 8, 2017

There's a patch available to fix this. @islandskater43 , if you want to merge #60 , I'll get started bringing my report downloading features over from my own fork.

@bmeynell
Copy link

bmeynell commented Sep 8, 2017

@ejegg - I've been using http://cs.sensiolabs.org/ recently to automate updating some older code to PSR-2. Take a look if you haven't already.

@ejegg
Copy link

ejegg commented Sep 8, 2017

@bmeynell, thanks for the suggestion! Do you find it better than phpcbf? I've used phpcbf for a few things, and found that it sometimes garbles comment text and omits newlines when inserting braces.

@ejegg
Copy link

ejegg commented Dec 15, 2017

Updated my reformat for 3.2.0 - now fixed by #63

@ejegg
Copy link

ejegg commented Jan 11, 2018

Hi @bjguillot , I've taken the 'elseif' clarifications out of my latest version of the PSR/2 fix so it should be easier to review. Is there any chance it could be merged this time?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants