Skip to content
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

Stub _n_noop, _nx_noop, translate_nooped_plural #131

Merged
merged 2 commits into from
Apr 19, 2023

Conversation

gmazzap
Copy link
Collaborator

@gmazzap gmazzap commented Dec 12, 2022

Stub for _n_noop(), _nx_noop(), and translate_nooped_plural() were missing in stubTranslationFunctions()

See #51

Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Verified against the WP functions and LGTM. Useful addition IMO.

I do see a risk with translate_nooped_plural potentially showing "undefined array index" notices, but I'm not concerned about that, as that would indicate a code error anyway, so if tests error out on that, all the better!

(Can I merge it or will you merge it yourself ?)

Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Argh... just realized tests are missing.... sorry about that. Could you add some ?
The FunctionsTest::testStubsTranslationsReturn() method seems the most appropriate place.

paulshryock added a commit to paulshryock/BrainMonkey that referenced this pull request Mar 1, 2023
@paulshryock
Copy link
Contributor

I added PR #133 which adds tests into this branch, to unblock this PR.

@paulshryock
Copy link
Contributor

paulshryock commented Mar 6, 2023

@gmazzap let me know if PR #133 needs anything else to be merged into this branch.

@gmazzap
Copy link
Collaborator Author

gmazzap commented Apr 19, 2023

Thanks, @jrfnl for the review.
Thanks @paulshryock for the PR.

I think this is good to go now.

@gmazzap gmazzap merged commit e99f227 into master Apr 19, 2023
@gmazzap gmazzap deleted the feature/complete-i18n-functions-stubs branch April 19, 2023 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants