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

Proper autoloading for composer, PSR-4 compliant, Cleanup #130

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gholol
Copy link

@gholol gholol commented May 15, 2016

Hi, I was little pissed off with using include directive each time I wanted to use this library, so I rewrote it.

It works the same way as before, but instead of include you have to use the use directive.

Tests are gonna fail (obviously). I don't found the time to rewrite them.

@gholol gholol mentioned this pull request May 15, 2016
jmleroux added a commit to jmleroux/geoPHP that referenced this pull request Feb 8, 2017
@@ -115,16 +109,16 @@ public function getBBox() {
$i++;
}

return array(
return [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

careful... the shorthand notation for array was only introduced in 5.4. The composer minimum php versions should be updated or keep it as array()

else return FALSE;
protected function beginsWith($str, $char)
{
return (substr($str,0,strlen($char)) == $char) ? true : false;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes automatically generated by your ide? Or are you doing them manually?

If manually - can we replace w/ ===?

@gsolak
Copy link

gsolak commented Nov 2, 2017

@gholol Chris, great work! This is a huge milestone for this project.

@phayes - Patrick can we please put this on the roadmap? I am happy to personally oversee the transition and rewriting the tests at some point.

I am thinking we should:

  1. Create a new tag and lock the current codebase at the current version (pre namespacing)
  2. Merge pull request
  3. Increment the major version and create a new tag w/ the namespaces.
  4. Update the README.md file w/ instructions how to install pre-namespaced code vs. new namespaced version.

While this approach isn't perfect -it does offers pretty good backwards compatibility.


Tests are very much important. But given how important namespacing is for a proper PHP project - I would be okay w/ making a one-off exception. I am currently using a namespaced version of this repo in production and all works 👍 .

@Soetens
Copy link

Soetens commented May 21, 2019

What's the waiting for? ;-)

@martsie
Copy link

martsie commented Dec 13, 2019

Will this be merged?

@@ -35,10 +35,11 @@ Example usage

```php
<?php
include_once('geoPHP.inc');

using Phayser\GeoPHP\GeoPHP;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
using Phayser\GeoPHP\GeoPHP;
using Phayes\GeoPHP\GeoPHP;

there's a typo there

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

Successfully merging this pull request may close these issues.

5 participants