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

Builder: make it immutable #979

Merged
merged 2 commits into from
Feb 21, 2023
Merged

Conversation

Slamdunk
Copy link
Collaborator

Fix #977

src/Token/Builder.php Show resolved Hide resolved
@Slamdunk Slamdunk changed the title Token\Builder: make it immutable Builder: make it immutable Nov 17, 2022
@Slamdunk Slamdunk requested a review from Ocramius November 17, 2022 14:56
@Ocramius Ocramius requested a review from lcobucci December 19, 2022 20:40
Copy link
Owner

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

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

Implementation looks good and it's great no signature had to be changed.

Let's document the change (also on the upgrading guide) and it's good to go. Thanks for helping out!

@Slamdunk Slamdunk requested a review from lcobucci December 20, 2022 07:37
tests/Token/BuilderTest.php Show resolved Hide resolved
@lcobucci
Copy link
Owner

I'll run the BC-break check locally against my PR that implements baseline to it and I believe we can merge this soon.

Since we're making the builder immutable, there's also no reason for having the "builder factory" on the configuration object. I'd say we can easily deprecate it on v5 and remove it on v6. I'll implement that on a separate PR.

@lcobucci
Copy link
Owner

I'll run the BC-break check locally against my PR that implements baseline to it and I believe we can merge this soon.

$ ../../ocramius/bc-check/bin/roave-backward-compatibility-check --from=origin/5.0.x
## 🇷🇺 Российским гражданам

В Украине сейчас идет война. Силами РФ наносятся удары по гражданской инфраструктуре в [Харькове][1], [Киеве][2], [Чернигове][3], [Сумах][4], [Ирпене][5] и десятках других городов. Гибнут люди - и гражданское население, и военные, в том числе российские призывники, которых бросили воевать. Чтобы лишить собственный народ доступа к информации, правительство РФ запретило называть войну войной, закрыло независимые СМИ и принимает сейчас ряд диктаторских законов. Эти законы призваны заткнуть рот всем, кто против войны. За обычный призыв к миру сейчас можно получить несколько лет тюрьмы.

Не молчите! Молчание - знак вашего согласия с политикой российского правительства.  
**Вы можете сделать выбор НЕ МОЛЧАТЬ.**

---

## 🇺🇸 To people of Russia

There is a war in Ukraine right now. The forces of the Russian Federation are attacking civilian infrastructure in [Kharkiv][1], [Kyiv][2], [Chernihiv][3], [Sumy][4], [Irpin][5] and dozens of other cities. People are dying – both civilians and military servicemen, including Russian conscripts who were thrown into the fighting. In order to deprive its own people of access to information, the government of the Russian Federation has forbidden calling a war a war, shut down independent media and is passing a number of dictatorial laws. These laws are meant to silence all those who are against war. You can be jailed for multiple years for simply calling for peace.

Do not be silent! Silence is a sign that you accept the Russian government's policy.  
**You can choose NOT TO BE SILENT.**

[1]: <https://cloudfront-us-east-2.images.arcpublishing.com/reuters/P7K2MSZDGFMIJPDD7CI2GIROJI.jpg> "Kharkiv under attack"
[2]: <https://gdb.voanews.com/01bd0000-0aff-0242-fad0-08d9fc92c5b3_cx0_cy5_cw0_w1023_r1_s.jpg> "Kyiv under attack"
[3]: <https://ichef.bbci.co.uk/news/976/cpsprodpb/163DD/production/_123510119_hi074310744.jpg> "Chernihiv under attack"
[4]: <https://www.youtube.com/watch?v=8K-bkqKKf2A> "Sumy under attack"
[5]: <https://cloudfront-us-east-2.images.arcpublishing.com/reuters/K4MTMLEHTRKGFK3GSKAT4GR3NE.jpg> "Irpin under attack"

#StandWithUkraine


Using "/home/lcobucci/projects/libs/jwt/.roave-backward-compatibility-check.json" as configuration file
Comparing from a7f5ad6d29ebf86dd639ed4e70112c9da7df5c5e to d53d7a5c4344277f4bb43fd17b496596b7dcd0f0...
Installing dependencies from lock file
Verifying lock file contents can be installed on current platform.
Package operations: 2 installs, 0 updates, 0 removals
  - Installing psr/clock (1.0.0): Extracting archive
  - Installing lcobucci/clock (3.0.0): Extracting archive
1 package you are using is looking for funding.
Use the `composer fund` command to find out more!
No security vulnerability advisories found
Installing dependencies from lock file
Verifying lock file contents can be installed on current platform.
Package operations: 2 installs, 0 updates, 0 removals
  - Installing psr/clock (1.0.0): Extracting archive
  - Installing lcobucci/clock (3.0.0): Extracting archive
1 package you are using is looking for funding.
Use the `composer fund` command to find out more!
No security vulnerability advisories found
No backwards-incompatible changes detected

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Builder fluent interface is confusing
4 participants