Skip to content

Conversation

simensen
Copy link
Contributor

In reference to mailing list conversation: https://groups.google.com/d/msg/php-fig/SJTL1ec46II/3DpYEzJ6Gz8J

  • Example namespace prefix has a trailing \ to show best practices for
    registering namespace prefixes to avoid Foo\Bar matching class Foo\BarBaz.

 * Example namespace prefix has a trailing `\` to show best practices for
   registering namespace prefixes to avoid `Foo\Bar` matching class `Foo\BarBaz`.
philsturgeon pushed a commit that referenced this pull request May 16, 2013
Fix General-Purpose Implementation example
@philsturgeon philsturgeon merged commit 58b9dfa into php-fig:master May 16, 2013
@pmjones
Copy link
Contributor

pmjones commented May 16, 2013

Jumped a little too quickly on that one @philsturgeon. Adding the \ at the end causes my test cases to fail (yes, I actually have a test for the proposal code ;-).

@pmjones
Copy link
Contributor

pmjones commented May 16, 2013

With that in mind I'm going to edit it back out again.

@philsturgeon
Copy link
Contributor

Woah there nelly!

Fix away, I just click green buttons.

@simensen
Copy link
Contributor Author

I have a test case for the other example but not this one. :) I also wasn't 100% sure this needed to be made at all, I only knew for sure that the other implementation needed to be changed.

Is the general-purpose implementation immune to the same problem the project-specific implementation suffered? If so, great. :)

One thing that would bother me is having to know whether or not the autoloader behind the scenes is going to require a trailing \ or if it is going to actually fail and/or otherwise not work if the trailing \ is added.

@pmjones
Copy link
Contributor

pmjones commented May 16, 2013

The general-purpose one is immune as far as I can tell. I suppose the GP implementation can strip trailing backslashes if needed, but I'd really rather modify the thing as little as possible at this point.

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.

3 participants