-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
UppercaseRot13Transformer wrong class name used #6714
Conversation
Regarding UppercaseRot13Transformer please double check this, because I suppose UppercaseTransformer has to be used instead. We are autowiring UppercaseTransformer, also there is no description of UppercaseRot13Transformer at all.
@@ -317,7 +317,7 @@ and a Twitter client using it: | |||
autowire: true | |||
|
|||
uppercase_rot13_transformer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rename this to uppercase_transformer
.
@jevgenijusr This change looks good to me. Can you also update the following code blocks for the XML and PHP formats accordingly? |
Removed usage of uppercase_rot13_transformer, used uppercase_transformer instead. Changed in YAML, XML and PHP code blocks.
👍 Status: Reviewed |
@jevgenijusr this looks like a bug. Thanks for fixing it! However, shouldn't we use |
Changed name of UppercaseTransformer to UppercaseRot13Transformer because it makes more sense: in this part of the article we do either rot13 transform or uppercased rot13 transform, therefore we use Rot13Transformer and UppercaseRot13Transformer. Actually the only problem this article had before this PR is it had mispelled "class UppercaseTransformer" (line 238) , it should have been "class UppercaseRot13Transformer", I renamed everything to use UppercaseTransformer at first, but best name would be UppercaseRot13Transformer (smallest change too) :) Take a look, thanks.
@javiereguiluz Thanks, I think we should use |
This PR was submitted for the 3.0 branch but it was merged into the 2.8 branch instead (closes #6714). Discussion ---------- UppercaseRot13Transformer wrong class name used Regarding UppercaseRot13Transformer: please double check this, because I suppose UppercaseTransformer has to be used instead. We are autowiring UppercaseTransformer, also there is no description of UppercaseRot13Transformer at all. P.S. If I am wrong, then I apologize 100 times:), but in any case this means it can be difficult to understand part of the recipe Commits ------- afa8e0e UppercaseRot13Transformer wrong class name used
Hi @jevgenijusr! Thanks for fixing this bug. The article is already quite complex, let alone when there is a typo in it :) I've merged your PR in the 2.8 version of the docs, as this feature was added in 2.8. I'll merge it in the newer versions from there. After merging, I decided to rename Thanks again! |
Regarding UppercaseRot13Transformer: please double check this, because I suppose UppercaseTransformer has to be used instead. We are autowiring UppercaseTransformer, also there is no description of UppercaseRot13Transformer at all. P.S. If I am wrong, then I apologize 100 times:), but in any case this means it can be difficult to understand part of the recipe