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

Localize remaining strings #198

Merged
merged 7 commits into from
Sep 3, 2019
Merged

Localize remaining strings #198

merged 7 commits into from
Sep 3, 2019

Conversation

hanzei
Copy link
Member

@hanzei hanzei commented Aug 15, 2019

This PR localizes the last few remaining strings. It does that by adding a poll.ErrorMessage struct (0/5 on the naming). This message is meant for a user to consume and should not be wrapped.

I would love to hear feedback on the design.

Fixes #196

@hanzei hanzei added 3. To Review Do Not Merge Should not be merged until this label is removed labels Aug 15, 2019
@hanzei hanzei added this to the v1.3.0 milestone Aug 15, 2019
@hanzei hanzei requested a review from kaakaa August 15, 2019 12:06
@hanzei hanzei self-assigned this Aug 15, 2019
@hanzei hanzei mentioned this pull request Aug 15, 2019
@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #198 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #198     +/-   ##
=========================================
+ Coverage   96.22%   96.33%   +0.1%     
=========================================
  Files          12       12             
  Lines         822      846     +24     
=========================================
+ Hits          791      815     +24     
  Misses         20       20             
  Partials       11       11
Impacted Files Coverage Δ
server/plugin/command.go 100% <100%> (ø) ⬆️
server/plugin/i18n.go 90.9% <100%> (+1.16%) ⬆️
server/plugin/api.go 93.67% <100%> (+0.02%) ⬆️
server/poll/poll.go 96.93% <100%> (+0.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a494b3...4ff092c. Read the comment docs.

server/poll/poll.go Outdated Show resolved Hide resolved
Copy link
Contributor

@kaakaa kaakaa left a comment

Choose a reason for hiding this comment

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

Current architecture is fine for me, and naming ErrorMessage is the best for now.
I left one more comment, so please take a look it.

server/plugin/api.go Outdated Show resolved Hide resolved
@hanzei hanzei requested a review from kaakaa August 17, 2019 05:19
server/plugin/command.go Outdated Show resolved Hide resolved
@hanzei hanzei requested a review from kaakaa August 17, 2019 16:37
@hanzei hanzei removed the Do Not Merge Should not be merged until this label is removed label Aug 19, 2019
@kaakaa kaakaa merged commit 3f17d0c into master Sep 3, 2019
@kaakaa kaakaa deleted the localize_errors branch September 3, 2019 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Localize remaining strings
2 participants