refactor(namespaces): renamed \Null to \NoEffect for future comp…

[norbertws]

…atibility with new PHP versions

'null' is a reserved keyword as of PHP version 7.0 and should not be used to name a class, interface or trait or as part of a namespace (T_NAMESPACE)

[makasim]

Thanks! I am a bit skeptical about the new name: NullTransporter.

Any other ideas ?

[norbertws]

Enqueue\DevNull or Enqueue\Void?

Just some other options came to my mind, but I'll think about it and we'll figure out a better name than NullTransporter. :)

[Steveb-p]

void has a high chance of becoming a namespace keyword in the future, if it's not already.

[norbertws]

Enqueue\Empty or Enqueue\Blank or maybe Enqueue\NoEffect? I'd vote for the last one, seems accurate and probably won't be reserved in the future.

[makasim]

Enqueue, EnqueueBundle and SimpleClient could be search for uses of null too.
There are factories and configurations for it.

[makasim]

NoEffect is ok for me.

[norbertws]

@makasim since enqueue's, amqp-ext's and a lot other packages' unit tests require "enqueue/null": "^0.10", which is the read-only most recent stable version of enqueue/null, am I right there should be 2 pull requests, one for the package null only and one for replacing namespaces everywhere else that will use it. What would be your approach?

I'm new to open-source and tagging, is there a way to have a version that ^0.10 won't pull down, but we can explicitly provide it (for example: 0.10-dev-only), in order to test the second PR and unit tests, so if that's all green, enqueue/null can get a new version and the the unit tests composer.json can use that from that on.

Is there a more elegant way to do this?

[Steveb-p]

@makasim since enqueue's, amqp-ext's and a lot other packages' unit tests require "enqueue/null": "^0.10", which is the read-only most recent stable version of enqueue/null, am I right there should be 2 pull requests, one for the package null only and one for replacing namespaces everywhere else that will use it. What would be your approach?

I'm new to open-source and tagging, is there a way to have a version that ^0.10 won't pull down, but we can explicitly provide it (for example: 0.10-dev-only), in order to test the second PR and unit tests, so if that's all green, enqueue/null can get a new version and the the unit tests composer.json can use that from that on.

Is there a more elegant way to do this?

Update this repository and this repository only.

This is called a mono-repository, that is automatically split by a script into sub-repositories. In other words, this repository is the source of truth for the small ones.

This approach is used to ensure that all packages at the current version are compatible with one another. A similar approach you can observe for example in Symfony, where there exists a single, main repository, which contains all packages and their tests, including cross-package tests.

[norbertws]

@Steveb-p got it, this repository only, but what am I missing? How would I let for example pkg/enqueue 's unit test know about the new namespaces if the stable version of pkg/null still uses Enqueue\Null namespace? I'm seeking advice, it's an interesting problem and I'm learning along the way. 🙂

[Steveb-p]

@Steveb-p got it, this repository only, but what am I missing? How would I let for example pkg/enqueue 's unit test know about the new namespaces if the stable version of pkg/null still uses Enqueue\Null namespace? I'm seeking advice, it's an interesting problem and I'm learning along the way. slightly_smiling_face

Everything was good with the initial changes, except for a typo. That's the primary reason the tests are failing - autoloader registers namespace incorrectly.

Beside that, we should consider the impact of changes namespace will have on the reliant packages. Technically, we're making a backward compatibility break here, because anything that relied on specific class name will break. I could propose using a class_alias + a leaving the namespace in composer.json (so there would be two entries pointing to the same folder). That could prevent us from introducing a BC break with downside that there would be no way of triggering a deprecation warning.

At least that's what I had to do in my workplace when doing a major namespace change once (well, it was similar, but the end result was equivalent).

[makasim]

You can introduce a new package enqueue\noeffect beside enqueue\null and migrate to it all other packages one by one.

That wont be a BC break because if someone wants to use null package it is still there.

Thoughts?

[Steveb-p]

You can introduce a new package enqueue\noeffect beside enqueue\null and migrate to it all other packages one by one.

That wont be a BC break because if someone wants to use null package it is still there.

Thoughts?

That's better actually, in this context. You're 💯 right.

[norbertws]

Excellent. I'll do it tomorrow.

[norbertws]

Please check the noeffect's README. My changes might not be the right ones, just replaced some links that will probably exist and added NoEffect in between Null and Transport. 🙂

[norbertws]

Any news guys? What's the process of reviewing this PR? When shall we expect it to go forward? (the company I'm working for is switching to 8.0 soon, that's one of the reasons I've made the change request)