Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[TwigBridge] allow null for $message of filter methodtrans#37941

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:5.1fromFlinsch:5.0
Aug 26, 2020
Merged

Conversation

@Flinsch
Copy link
Contributor

QA
Branch?5.0
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#37931
LicenseMIT

With Symfony 5.0, filter methodtrans of Symfony Twig Bridge does not allow null values for$message parameter anymore, breaking backward compatibility. See also#37931. The included commit provides a fix to this BC break by allowing null values again.

@fabpot
Copy link
Member

Closing as explained in the issue.

@fabpotfabpot closed thisAug 25, 2020
@Flinsch
Copy link
ContributorAuthor

Closing as explained in the issue.

Have you even read the issue in full,@fabpot? Sorry, I don't mean to be rude, but I can't understand how you can just close this without even considering.

@fabpot
Copy link
Member

Reopening

Flinsch reacted with thumbs up emoji

@fabpotfabpot reopened thisAug 26, 2020
@nicolas-grekasnicolas-grekas added this to the5.1 milestoneAug 26, 2020
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Works for me, we did this already in the Translator class.
Please add a test case.

Flinsch reacted with thumbs up emoji
@nicolas-grekasnicolas-grekas changed the base branch from5.0 to5.1August 26, 2020 12:22
@nicolas-grekasnicolas-grekas changed the base branch from5.1 to5.0August 26, 2020 12:22
@nicolas-grekas
Copy link
Member

Can you please rebase and target 5.1 also?

@Flinsch
Copy link
ContributorAuthor

Will do. Thank you for your consideration.

@nicolas-grekasnicolas-grekas changed the title[TwigBridge] Fix #37931: BC break where filter methodtrans did not allow null values for $message parameter anymore[TwigBridge] allow null values for $message of filter methodtransAug 26, 2020
@nicolas-grekasnicolas-grekas changed the title[TwigBridge] allow null values for $message of filter methodtrans[TwigBridge] allow null for $message of filter methodtransAug 26, 2020
@Flinsch
Copy link
ContributorAuthor

Flinsch commentedAug 26, 2020
edited
Loading

Can you please rebase and target 5.1 also?

@nicolas-grekas, the source branch cannot be changed for a pull request. I can rebase 5.0 onto 5.1 but the source branch will still read 5.0 which might lead to confusions. What should I do in your opinion? Should we stick to the name 5.0 for the source branch or should I replace the pull request with a new one having target/base branch and source branch both set to 5.1?

The Symfony guidelines on how to create pull requests say that "bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.)" That's why I have chosen 5.0 in the first place. Just as a remark. I'm sorry if that was wrong.

@nicolas-grekas
Copy link
Member

You can rebase your branch 5.0 on 5.1 yes, that'd be fine on our side. You could then delete it on your fork, 5.0 is not maintained anymore anyway.

Flinsch reacted with thumbs up emoji

@Flinsch
Copy link
ContributorAuthor

Flinsch commentedAug 26, 2020
edited
Loading

Please add a test case.

Done.

fabbot.io still has comlaints about "Common Typos" and "Merge Commits" which don't seem to have any relations to my changes/commits however.

@fabpot
Copy link
Member

Looks like you forgot to rebase your changes on 5.1 (many unrelated commits). When done, push it again, that should be enough.

Flinsch reacted with thumbs up emoji

… allow null values for `$message` parameter anymore
@nicolas-grekas
Copy link
Member

Thank you@Flinsch.

Flinsch reacted with thumbs up emoji

@nicolas-grekasnicolas-grekas merged commit9c86cd2 intosymfony:5.1Aug 26, 2020
@Flinsch
Copy link
ContributorAuthor

Thank you@Flinsch.

I can only return my thanks,@nicolas-grekas.

@fabpotfabpot mentioned this pull requestAug 31, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@dunglasdunglasAwaiting requested review from dunglas

@jderussejderusseAwaiting requested review from jderusse

@lyrixxlyrixxAwaiting requested review from lyrixx

@srozesrozeAwaiting requested review from sroze

@xabbuhxabbuhAwaiting requested review from xabbuh

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

Assignees

No one assigned

Projects

None yet

Milestone

5.1

Development

Successfully merging this pull request may close these issues.

4 participants

@Flinsch@fabpot@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp