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

[tapocontrol] Move error messages to i18n #14790

Merged
merged 2 commits into from
May 5, 2023
Merged

Conversation

wildcs
Copy link
Contributor

@wildcs wildcs commented Apr 12, 2023

before this pr error messages were declared as constant.
moved them to i18n

@wildcs wildcs force-pushed the tapocontrol branch 2 times, most recently from 45ca75f to 518fe20 Compare April 17, 2023 16:54
@wildcs wildcs changed the title [tapocontrol] Defined error messages in i18n [tapocontrol] Moved error messages to i18n Apr 17, 2023
@lolodomo
Copy link
Contributor

Your approach looks strange. If you want to provide generic messages with a number as parameter, you do not need to duplicate entries to be translated. Create one with a parameter, something like received error code ({0})

@lolodomo
Copy link
Contributor

lolodomo commented Apr 22, 2023

And your way to retrieve the constant name to get the i18n entry is not very good.
Why not simply create an enum type containing all your errors ? Then you just have to retrieve the enum name from an int value.
Something like (partial code):

enum TapoError {
    ERR_API_COMMON_FAILED(-1),
    ERR_API_SESSION_TIMEOUT(9999),

...

    int code;
   TapoError(int code) {
       this.ocde = code;
    }

    static TapoError fromCode(int code) throws xxx {
        ...
    }
}

Then your i18n entry can be built using TapoError.fromCode(code).name().replaceFirst("ERR_", "error-").replace("_", "-").toLowerCase()

@wildcs
Copy link
Contributor Author

wildcs commented Apr 30, 2023

Thanks for your hint. Changed it and hope it's okay now.

@wildcs wildcs requested a review from lolodomo April 30, 2023 18:29
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

Looks clearly better.

@lolodomo lolodomo added i18n enhancement An enhancement or new feature for an existing add-on labels May 3, 2023
moved constants from class to enum

Signed-off-by: Christian Wild <[email protected]>
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo lolodomo merged commit 58470ca into openhab:main May 5, 2023
@lolodomo lolodomo added this to the 4.0 milestone May 5, 2023
@lolodomo lolodomo changed the title [tapocontrol] Moved error messages to i18n [tapocontrol] Move error messages to i18n May 5, 2023
@wildcs wildcs deleted the tapocontrol branch May 5, 2023 21:57
tb4jc pushed a commit to tb4jc/openhab-addons that referenced this pull request Jun 19, 2023
* [tapocontrol] Moved error messages to i18n

---------

Signed-off-by: Christian Wild <[email protected]>
Signed-off-by: Thomas Burri <[email protected]>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
* [tapocontrol] Moved error messages to i18n

---------

Signed-off-by: Christian Wild <[email protected]>
Signed-off-by: Matt Myers <[email protected]>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
* [tapocontrol] Moved error messages to i18n

---------

Signed-off-by: Christian Wild <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on i18n
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants