Skip to content

[Web Link] replace for all functions - #1032

Closed
JohJohan wants to merge 1 commit into
rectorphp:mainfrom
JohJohan:WebLink
Closed

[Web Link] replace for all functions#1032
JohJohan wants to merge 1 commit into
rectorphp:mainfrom
JohJohan:WebLink

Conversation

@JohJohan

Copy link
Copy Markdown
Contributor

I am adding function replacement for 6.3 for withRel and withoutRel as well.

Q: I use a foreach to replace the functions is that okay?

@TomasVotruba

Copy link
Copy Markdown
Member

This is a bit mess to review, but it seems all 3 methods have the same changes.

Can you link me to PR/blog post in Symfony?

What if we allow multiple method names in the config value object?

@JohJohan

Copy link
Copy Markdown
Contributor Author

This is a bit mess to review, but it seems all 3 methods have the same changes.

Can you link me to PR/blog post in Symfony?

What if we allow multiple method names in the config value object?

Yeah i can understand that, indeed: __construct, withRel and withoutRel all can use the constant instead of string value.

Here is some documentation: https://symfony.com/doc/current/web_link.html#:~:text=The%20possible%20values%20of%20link%20relations%20(%27preload%27%2C%20%27preconnect%27%2C%20etc.)%20are%20also%20defined%20as%20constants%20in%20the%20Link%20class%20(e.g.%20Link%3A%3AREL_PRELOAD%2C%20Link%3A%3AREL_PRECONNECT%2C%20etc.).

You mean ReplaceArgumentDefaultValue would allow second argument to be a array of methods? Its a option but this foreach also works what do you prefer?

@TomasVotruba

Copy link
Copy Markdown
Member

I mean something like upgrade/chagelong. That's the source we work with.
There is no withoutRel/withRel in the docs.

@JohJohan

Copy link
Copy Markdown
Contributor Author

is no withoutRel/withRel in the doc

Ah i cannot really find a good source but found: symfony/web-link@034ff81#diff-f82e9bb0d7c54260c4db845cdeb8d2f1e46e540332b0244a8e9476ea273bffb3R31-R119 where the Link class was added already containing these methods and option to pass a string, and also constants to use so its there since the beginning

@JohJohan

Copy link
Copy Markdown
Contributor Author

@TomasVotruba the code before was also missing the link to the docs, and there isn't really documentation for this its part of the PSR-13 Link EvolvableLinkInterface.

You think its okay to merge link this or want me to add some more information links?

@TomasVotruba

Copy link
Copy Markdown
Member

As there is no change in Symfony, this looks like something that should be handled by custom rule.
Create and use custom rule instead.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants