Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[Translator] Option to throw an exception instead of returning the message as is when the message is not found. #2185

Merged
merged 1 commit into from
Sep 23, 2024
Merged

[Translator] Option to throw an exception instead of returning the message as is when the message is not found. #2185

merged 1 commit into from
Sep 23, 2024

Conversation

Crovitche-1623
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
Issues Fix #2183
License MIT

@carsonbot carsonbot added Feature New Feature Status: Needs Review Needs to be reviewed labels Sep 19, 2024
Kocal
Kocal reviewed Sep 19, 2024
View reviewed changes
Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

Please see #2183 (comment) for discussion / the implementation details

Kocal
Kocal reviewed Sep 19, 2024
View reviewed changes
src/Translator/assets/src/translator.ts Outdated Show resolved Hide resolved
src/Translator/assets/src/translator.ts Outdated Show resolved Hide resolved
src/Translator/assets/src/translator.ts Outdated Show resolved Hide resolved
src/Translator/assets/src/translator.ts Outdated Show resolved Hide resolved
@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Sep 19, 2024
@Kocal
Copy link
Member

Kocal commented Sep 19, 2024

Hum, my bad, in fact, we totally don't need to update the configuration file at all.

The function throwWhenNotFound() must be explicitly called by developers if they want to, this can be done in the assets/translator.js file.

Also, the documentation src/Translator/doc/index.rst must be updated too!

@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Sep 19, 2024
@Crovitche-1623
Copy link
Contributor Author

Now I consider my PR ready @Kocal. Let me know what you think.

Kocal
Kocal reviewed Sep 20, 2024
View reviewed changes
src/Translator/assets/test/translator.test.ts Show resolved Hide resolved
src/Translator/assets/test/translator.test.ts Outdated Show resolved Hide resolved
src/Translator/assets/test/translator.test.ts Outdated Show resolved Hide resolved
src/Translator/doc/index.rst Outdated Show resolved Hide resolved
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Sep 21, 2024
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Reviewed Has been reviewed by a maintainer labels Sep 23, 2024
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Sep 23, 2024
@Kocal
Copy link
Member

Kocal commented Sep 23, 2024

Thanks for your work on this new feature!

@Kocal Kocal merged commit 44f6b48 into symfony:2.x Sep 23, 2024
60 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@smnandre smnandre smnandre approved these changes

@Kocal Kocal Kocal approved these changes

Assignees
No one assigned
Labels
Feature New Feature Status: Reviewed Has been reviewed by a maintainer Translator
Projects
None yet
Milestone
No milestone
4 participants
@Crovitche-1623 @Kocal @smnandre @carsonbot