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

Remove t from system console code #27054

Merged
merged 9 commits into from
May 27, 2024
Merged

Remove t from system console code #27054

merged 9 commits into from
May 27, 2024

Conversation

hmhealey
Copy link
Member

@hmhealey hmhealey commented May 17, 2024

Summary

This is part of my mission to eradicate t from last week. Most of it is pretty straightforward replacement of t with defineMessage/defineMessages, and I also occasionally removed some use of localizeMessage as well, but I'll comment on anything noteworthy below.

Ticket Link

https://mattermost.atlassian.net/browse/MM-58324

Release Note

NONE

@hmhealey hmhealey added the 2: Dev Review Requires review by a developer label May 17, 2024
@mm-cloud-bot mm-cloud-bot added the release-note-none Denotes a PR that doesn't merit a release note. label May 17, 2024

copyID: string;
copyDefault: string;
title: MessageDescriptor;
Copy link
Member Author

Choose a reason for hiding this comment

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

A lot of these changes are replacing ID/default props with a single prop that takes a message descriptor

let convertMessage;
if (toPublic) {
convertMessage = (
<FormattedMarkdownMessage
Copy link
Member Author

Choose a reason for hiding this comment

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

There's still some things here that would be worth fixing such as removing FormattedMarkdownMessage and fixing some weird whitespace in this string that the FormatJS ESLint plugin will complain about, but this diff is difficult enough to read as-is, so I'm not changing it yet

@@ -66,9 +67,21 @@ const GroupList = ({
<AbstractList
header={<Header/>}
renderRow={renderRow}
emptyListText={isModeSync ? messages.emptyListModeSync : messages.emptyList}
Copy link
Member Author

Choose a reason for hiding this comment

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

This used to be done in the index file for AbstractList which seemed really odd to me and was confusing TypeScript's ability to check the type of props, so I moved it here to make that easier

Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

Great work! Just a few nitpicks.

@hmhealey hmhealey added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels May 27, 2024
@hmhealey hmhealey self-assigned this May 27, 2024
@hmhealey hmhealey merged commit 2f7f00b into master May 27, 2024
24 checks passed
@hmhealey hmhealey deleted the hh_may17-t-2 branch May 27, 2024 19:49
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation release-note-none Denotes a PR that doesn't merit a release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants