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

PSR1 and PSR2 Conversion #319

Closed
wants to merge 6 commits into from
Closed

PSR1 and PSR2 Conversion #319

wants to merge 6 commits into from

Conversation

Braunson
Copy link
Contributor

Reference issue #317.

Tried to run tests on current master copy in addition to this PR version, kept getting errors referencing Class SendGrid and Class SendGrid\Client not found. I'm going to assume it's related to this #91

With the flags: align_double_arrow,short_echo_tag,line_after_namespace,long_array_syntax,no_blank_lines_before_namespace,ordered_use
With the flags: align_double_arrow,short_echo_tag,line_after_namespace,long_array_syntax,no_blank_lines_before_namespace,ordered_use
@thinkingserious thinkingserious added type: community enhancement feature request not on Twilio's roadmap status: cla needed status: work in progress Twilio or the community is in the process of implementing labels Oct 25, 2016
@thinkingserious
Copy link
Contributor

Hello @Braunson,

This is awesome! Thanks for your contribution!

I will look into why the tests are failing.

Could you please take a moment to sign our CLA so I can merge your changes?

Thanks!

@thinkingserious
Copy link
Contributor

CLA received, thanks!

@Dreyer
Copy link
Contributor

Dreyer commented Nov 28, 2016

Definitely on board for ditching PEAR for PHP FIG recommendations for coding style conventions.

Although, it looks like this PR, is reverting from the short array syntax back to the long array syntax. From what I can work out, PSR-2 does not seem to make an explicit decision on the short array syntax. However, they do use it in examples in Chapter 4.3.

According to the PHP docs, unless the intention is for the library to have backward compatibility to support PHP < 5.4, there doesn't seem to be much benefit.

@thinkingserious
Copy link
Contributor

@Dreyer,

Thanks for taking the time to provide some feedback!

We do not support versions < 5.4 and do not plan to. That said, the long array syntax does not seem to hurt anything and would probably be helpful to those who must run a version < 5.4.

@Braunson,

Do you have a strong opinion on why we should revert to the long array syntax?

@Braunson
Copy link
Contributor Author

Braunson commented Nov 30, 2016

@thinkingserious I'd actually prefer the shorter array syntax but as you said the longer is compatible for people who must run < 5.4.

However according to current stats, the usage as of May 2016 for 5.4 and below is ~8% and rapidly dropping.

I'll adjust the PR to the shorter syntax!

Edit: Done 👍

@SendGridDX
Copy link

Hello @Braunson,

This issue has finally risen to the top of our queue!

I notice that we have a conflicting file, could you please resolve?

Thanks!

Elmer

@Braunson
Copy link
Contributor Author

Braunson commented Mar 31, 2017

@SendGridDX I'm not sure if I can edit this PR anymore, the original fork was deleted.

Braunson wants to merge 6 commits into sendgrid:master from unknown repository

Edit: Seems this PR is an orphan now 😞

@Braunson Braunson mentioned this pull request Mar 31, 2017
@Braunson
Copy link
Contributor Author

@SendGridDX Since I can't edit this PR, I've created a new one from a new fork with the same changes, it should pass tests. You can find the new PR here #373 . Probably best to close this PR now.

@Braunson Braunson closed this Mar 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
4 participants