Skip to content

Conversation

RafaelKr
Copy link

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets -
Related issues/PRs -
License MIT
Documentation PR -

What's in this PR?

Use Uuid::v7()->toRfc4122() instead of Uuid::v7()->__toString().

Why?

Both methods output the $this->uid property, but toRfc4122 also describes the returned format (e.g. f81d4fae-7dec-11d0-a765-00a0c91e6bf6)

@mamazu mamazu added the DX Affecting the end developer label Jan 8, 2025
@alexander-schranz
Copy link
Member

@RafaelKr can you try to repush something. The CI seems not be triggered not sure if the fork is corrupt or something else going wrong.

@RafaelKr RafaelKr force-pushed the refactor/uuid-toRfc4122 branch from 68d6973 to 0388672 Compare January 9, 2025 19:13
@RafaelKr RafaelKr force-pushed the refactor/uuid-toRfc4122 branch from 0388672 to cd30bf3 Compare January 9, 2025 19:15
@RafaelKr
Copy link
Author

RafaelKr commented Jan 9, 2025

@alexander-schranz I just rebased on 3.0 and force-pushed

Edit: It says "This workflow requires approval from a maintainer."

@alexander-schranz
Copy link
Member

alexander-schranz commented Jan 9, 2025

Looks like the "new github merge experience" is missing that button had to disable the new merge UI now I did see the button again. Thx for letting us know @RafaelKr .

@alexander-schranz
Copy link
Member

@mamazu we may should add a rector.php rename method rule Uuid class __toString() -> toRfc4122 to keep our code base consistent and not again use accidently __toString: https://getrector.com/rule-detail/rename-method-rector

@alexander-schranz alexander-schranz merged commit 89f3c02 into sulu:3.0 Jan 30, 2025
9 checks passed
@alexander-schranz
Copy link
Member

@RafaelKr thank you!

@RafaelKr RafaelKr deleted the refactor/uuid-toRfc4122 branch January 30, 2025 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Affecting the end developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants