-
Notifications
You must be signed in to change notification settings - Fork 310
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
Refactor some class components to functional #6712
Conversation
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.
Great work and a lot of changes indeed. Let's see what the e2es say.
I've found some minor issues.
@ryaplots please take a look as well |
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.
Nice work. Just a small issue with the pubsub form according to the e2e tests.
See my remark.
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.
Thank you. Please clean up the commit trail before merging. Any fix commits should be squashed into their respective earlier commit. You can use interactive rebasing to achieve that.
2f84769
to
1ad3498
Compare
Summary
References #6376
Changes
List of refactored components:
Notes for Reviewers
When testing, please pay more attention to the refactored form components because they are large and you know the business logic more briefly(if everything behaves as expected).
Checklist
README.md
for the chosen target branch.CHANGELOG.md
.CONTRIBUTING.md
, there are no fixup commits left.