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

[Console] Revert StringInput bc break from #45088#45240

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
fancyweb merged 1 commit intosymfony:4.4frombobthecow:console-stringinput
Jan 31, 2022

Conversation

@bobthecow
Copy link
Contributor

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFixbobthecow/psysh#703
LicenseMIT
Doc PR-

Fix a backwards compatibility break introduced in#45088 and released in v4.4.37, v5.3.14, v5.4.3 and v6.0.3.

I went withREGEX_UNQUOTED_STRING by analog to theREGEX_QUOTED_STRING constant name, but don't have a strong opinion on what it should be called :)

@nicolas-grekas
Copy link
Member

Are you using the constant and the changed value breaks your stuff? What about making the new constant private instead?

@bobthecow
Copy link
ContributorAuthor

PsySH extends StringInput for its ShellInput (which basically adds the ability to have a "code" chunk that gets treated literally). It had to implement its owntokenize method and was referencing these two constants, since it had so much in common.

If you don't want this to be part of the public API, it'd make sense to make the new one private. I don't think anyone else was silly enough to use it, but there are a lot of PsySH versions going back years that depend on the previous public constants so let's at least keep them working :)

@nicolas-grekas
Copy link
Member

Thanks for the PR and the details. I'm fine as is :)

bobthecow reacted with thumbs up emoji

Copy link
Contributor

@fancywebfancyweb left a comment

Choose a reason for hiding this comment

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

Is the constant really deprecated though? Just adding a comment won't be enough I guess.

@bobthecow
Copy link
ContributorAuthor

Even if it were possible (I'm not sure it is) any more aggressive deprecation than a comment would violate semver.

@fancyweb
Copy link
Contributor

I mean in 6.1. Either we don't deprecate it at all and we remove that comment or we need a strategy on 6.1 to effectively be able to remove the constant in 7.0. I just don't remember if just adding a@deprecated comment is enough for a constant.

@bobthecow
Copy link
ContributorAuthor

I'm not sure what you normally do, but I found several other comment-only deprecation notes that I modeled this after.

@fancyweb
Copy link
Contributor

Thank you@bobthecow.

@fancywebfancyweb merged commit1276d32 intosymfony:4.4Jan 31, 2022
@bobthecow
Copy link
ContributorAuthor

Thank you!

@bobthecowbobthecow deleted the console-stringinput branchFebruary 28, 2022 16:38
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

+1 more reviewer

@fancywebfancywebfancyweb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@bobthecow@nicolas-grekas@fancyweb@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp