Skip to content

fix typo ($tuple not $tupel)#38569

Merged
wilsonge merged 1 commit intojoomla:4.2-devfrom
sba:patch-1
Sep 6, 2022
Merged

fix typo ($tuple not $tupel)#38569
wilsonge merged 1 commit intojoomla:4.2-devfrom
sba:patch-1

Conversation

@sba
Copy link
Copy Markdown
Contributor

@sba sba commented Aug 23, 2022

In german Tupel would be correct, but in english the variable has to be named "tuple".

In german Tupel would be correct, but in english the variable has to be named "tuple".
@brianteeman
Copy link
Copy Markdown
Contributor

the suggested change to english is correct. I dont know if there are any consequences of renaming this variable. when I tried to correct a variable name before it was rejected.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 23, 2022

Hmm do you have the link to the PR that you did for that or remember the reason to redject it? Its a variable used only in the save method so I dont see an issue here on a quick look. Technical it could be argued that such a change in a layout or variable used within an layout could be a B/C issue but looks like the github search does only find "tupel" within other two unrelated issues and this PR here https://github.com/joomla/joomla-cms/search?q=tupel&type=issues

@brianteeman
Copy link
Copy Markdown
Contributor

(#29812 #36821)

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 23, 2022

The first PR was closed by the person who opend the PR and not redjected. The second PR is kind of a b/c issue as benjamin mentiond there. This PR here should be fine from this perspective.

@brianteeman
Copy link
Copy Markdown
Contributor

I would have remade the first one but didnt because of the answer to the second one

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 23, 2022

The first one should not have a b/c issue but I'm not 100% sure with that sentence by @HLeithner there. But this here is "just" the variable name thats not used within layouts and not used in signatures so this should be fine. The seccond one is not the variable name but the content of an variable. As J5 is open both PRs could be made against it to get it fixed finally.

@HLeithner
Copy link
Copy Markdown
Member

HLeithner commented Aug 23, 2022

No idea why there is such a long discussion the variable is only in local scope and can be changed to whatever you want. This can be merged into 4.2.2 from my point of you.

@ChristineWk
Copy link
Copy Markdown

I have tested this item ✅ successfully on 2929f1e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38569.

1 similar comment
@viocassel
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 2929f1e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38569.

@alikon
Copy link
Copy Markdown
Contributor

alikon commented Aug 24, 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38569.

@joomla-cms-bot joomla-cms-bot added RTC This Pull Request is Ready To Commit and removed PR-4.2-dev labels Aug 24, 2022
@wilsonge wilsonge merged commit ca20806 into joomla:4.2-dev Sep 6, 2022
@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented Sep 6, 2022

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 6, 2022
@wilsonge wilsonge added this to the Joomla! 4.2.3 milestone Sep 6, 2022
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.

9 participants