-
Notifications
You must be signed in to change notification settings - Fork 362
msglist: Update label to 'Messages with yourself' in DM header #1323
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
Conversation
53dd04c to
fefccea
Compare
chrisbobbe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i18n: Update label to "Messages with yourself" in DM header
commit-message nit: choose a different prefix than i18n. This commit involves the i18n subsystem, because it's changing a UI string and we translate those—but it isn't a change in the i18n subsystem itself. How about msglist.
|
Also, updating the PR title to remove |
fefccea to
03a972b
Compare
|
Thanks, LGTM! Marking for Greg's review. |
gnprice
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this! A few small comments.
There's a TODO comment where this is used:
} else {
// TODO pick string; web has glitchy "You and $yourname"
title = zulipLocalizations.messageListGroupYouWithYourself;We're taking care of that TODO with this change, so the comment should be deleted.
There was a chat thread where we decided to make this change, right? Let's have the commit message link to that thread.
In the commit message:
Fixes: zulip#1319
It should say just #1319, not zulip#1319 — the "zulip" part is redundant.
a8eaa7f to
81feafd
Compare
81feafd to
1e8397b
Compare
|
Thanks for the review @gnprice, pushed the revision, PTAL. |
|
Thanks for the revision! Looks good; merging. |
Fixes: #1319
Screenshot