Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Remove create-react-class #5157

Merged
merged 11 commits into from
Sep 3, 2020
Merged

Remove create-react-class #5157

merged 11 commits into from
Sep 3, 2020

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Aug 29, 2020

No description provided.

@t3chguy t3chguy requested a review from a team August 29, 2020 11:19
@t3chguy t3chguy marked this pull request as draft August 29, 2020 12:01
@t3chguy t3chguy marked this pull request as ready for review August 29, 2020 17:37
@turt2live
Copy link
Member

This isn't against develop?

@t3chguy
Copy link
Member Author

t3chguy commented Aug 31, 2020

It's staged against my other pr which removes stale components

Base automatically changed from t3chguy/crc to develop September 1, 2020 08:12
Copy link
Contributor

@JorikSchellekens JorikSchellekens left a comment

Choose a reason for hiding this comment

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

Damn, that was thorough. I'm so happy about this.

It largely looks good to me. Just a few questions about the continued use of UNSAFE react lifecycle methods. And a few other curiosities.

I found it really hard to verify that all methods were correctly converted to either unbound functions or closures because of GH's review ui. However, the majority of cases were intuitive.

I'm happy with it but I guess we may also wait for some more eyes, such at turt2live's?

src/components/structures/RoomView.js Show resolved Hide resolved
src/components/structures/TagPanel.js Show resolved Hide resolved
src/components/structures/TimelinePanel.js Show resolved Hide resolved
src/components/structures/TimelinePanel.js Show resolved Hide resolved
src/components/views/auth/RegistrationForm.js Show resolved Hide resolved
src/components/views/elements/EditableText.js Outdated Show resolved Hide resolved
src/components/views/rooms/MemberList.js Outdated Show resolved Hide resolved
src/components/views/settings/ChangeAvatar.js Show resolved Hide resolved
Copy link
Contributor

@JorikSchellekens JorikSchellekens left a comment

Choose a reason for hiding this comment

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

Looks great, so nice to be rid of this

Copy link
Contributor

@JorikSchellekens JorikSchellekens left a comment

Choose a reason for hiding this comment

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

And therefore I approve of it

@t3chguy t3chguy merged commit e624ce1 into develop Sep 3, 2020
@t3chguy t3chguy deleted the t3chguy/crc1 branch September 3, 2020 16:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants