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

[UID] Rename NullUuid to NilUuid #36075

Merged
merged 1 commit into from Mar 15, 2020
Merged

[UID] Rename NullUuid to NilUuid #36075

merged 1 commit into from Mar 15, 2020

Conversation

@ro0NL
Copy link
Contributor

ro0NL commented Mar 14, 2020

Q A
Branch? master
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

As per https://tools.ietf.org/html/rfc4122#section-4.1.7

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 14, 2020
@nicolas-grekas nicolas-grekas requested a review from javiereguiluz Mar 14, 2020
@javiereguiluz

This comment has been minimized.

Copy link
Member

javiereguiluz commented Mar 14, 2020

👎 I disagree with this. We use PHP ... so null is natural and nil is alien to us.

The fact that the RFC uses that terminology is not enough reason for me. The RFC also uses "node" instead of "mac address" ... and we don't comply with the RFC and keep calling it "mac address".

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

ro0NL commented Mar 14, 2020

The fact that the RFC uses that terminology is not enough reason for me.

for me it was actually + the fact you get a slightly more relevant search result

Agree it's minor.. and if both are considerable equal in practice im fine with "wont fix"

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Mar 14, 2020

This is worth considering as this is what the RFC uses, thanks for raising the point. I'm also thinking our community is not used to "nil" so it could create confusion where "null" wouldn't. Dunno if others agree?

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

ro0NL commented Mar 14, 2020

See also https://english.stackexchange.com/questions/61363/are-nil-and-null-interchangeable, esp.:

Nil means zero or nought
Null means nothing or empty

given "a nil/null uuid" is something, it made me lean towards "nil" being the better name

@wouterj

This comment has been minimized.

Copy link
Member

wouterj commented Mar 14, 2020

I agree with @ro0NL here. The null we as PHP developers know is not a zero or an empty string, it's the non-existence of a value. So the null in NullUuid does not relate to the null we know in PHP.

In Symfony's Notifier component, such a class is called NoRecipient (and not NullRecipient).

I think it makes sense to follow the RFC here 👍

(Yet, if I'm completely honest, we use the Null prefix in many other classes)

Copy link
Member

nicolas-grekas left a comment

Nil means zero or nought
Null means nothing or empty

that's quite convincing I agree.
Good to me.

@fabpot
fabpot approved these changes Mar 15, 2020
@fabpot fabpot force-pushed the ro0NL:nil-uuid branch from d9668eb to cbb6d23 Mar 15, 2020
@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Mar 15, 2020

Thank you @ro0NL.

@fabpot fabpot merged commit 1d955cc into symfony:master Mar 15, 2020
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details
@ro0NL ro0NL deleted the ro0NL:nil-uuid branch Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.