🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@JohJohan
Copy link
Contributor

@TomasVotruba
Copy link
Member

TomasVotruba commented Sep 14, 2022

Thanks 👍

What Symfony version support the name and the description?
As I read the Symfony blog, it's 2 different versions, thus 2 different configs and rules.

@JohJohan
Copy link
Contributor Author

I think from 5.3 they support this: https://symfony.com/doc/5.3/console.html (but that is not maintained) so i would say 5.4/6.0 and 6.1

The blog post you probably found: https://symfony.com/blog/new-in-symfony-5-3-lazy-command-description

What do you mean by

thus 2 different configs and rules.

Should i use different configs to test this behavior? And is there some docs about that?

@TomasVotruba
Copy link
Member

TomasVotruba commented Sep 14, 2022

Symfony 5.3 is for the $defaultDescription, yes 👍

The $defaultName is available since Symfony 3.4: symfony/symfony#23887


Should i use different configs to test this behavior? And is there some docs about that?

The docs can be found here: https://github.com/rectorphp/rector/tree/main/docs

But there is no description rule yet, AFAIK. So the new rule and tests are needed.

@JohJohan
Copy link
Contributor Author

Ah like that i can do that but takes a bit more time

final class SunshineCommand extends Command
{
protected static $defaultName = 'sunshine';
protected static $defaultDescription = 'sunshine description';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to force this order? Right now its adding the defaultDescription as first property.
So first $defaultName and then $defaultDescription

Copy link
Member

Choose a reason for hiding this comment

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

You'd have to look for existing properties on the Class_ and if there's a property of "defaultName", add a new property after it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added function addDefaultDescriptionProperty to do it, you may resolve this if it is okay

@samsonasik
Copy link
Member

You can run:

vendor/bin/rector && composer fix-cs 

To make CI green.

@JohJohan
Copy link
Contributor Author

Thanks @samsonasik i did and squashed my commits into 1 commit.

@samsonasik
Copy link
Member

String_ and ClassConstFetch check no longer needed, can be removed as already checked by ExprAnalyzer.

Also, please update PR Title to something like:

[Symfony53] Add CommandDescriptionToPropertyRector

@JohJohan JohJohan changed the title 229 Add fixtures to check description is correctly placed as property [Symfony53] Add CommandDescriptionToPropertyRector Sep 15, 2022
Add a rule to move command description setter to property
@JohJohan
Copy link
Contributor Author

Check, thanks for the feedback 👍

@TomasVotruba
Copy link
Member

Thank you 👏

@TomasVotruba TomasVotruba merged commit 113df7c into rectorphp:main Sep 19, 2022
@JohJohan JohJohan deleted the 229 branch October 2, 2022 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants