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

refactor: convert rhs_sidebar component to ts #527

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hereje
Copy link

@hereje hereje commented Oct 1, 2024

Summary

refactor: convert rhs_sidebar component to ts

  • convert component to ts
  • update types

Ticket Link

Fixes: #425

@wiggin77 wiggin77 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Oct 1, 2024
Copy link
Contributor

@raghavaggarwal2308 raghavaggarwal2308 left a comment

Choose a reason for hiding this comment

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

@hereje Mostly looks good to me. Just one small comment for discussion.

webapp/src/components/rhs_sidebar/rhs_sidebar.tsx Outdated Show resolved Hide resolved
- convert component to ts
- update types

Fixes: mattermost#425
@hereje hereje force-pushed the refactor/convert-rhs_sidebar-to-ts branch from 9c97a43 to 1696ffd Compare October 10, 2024 02:47
Copy link
Contributor

@raghavaggarwal2308 raghavaggarwal2308 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution!

Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

Thanks @hereje. There are some linter issues with this PR. Will you be able to address?

- update props types as optional according to original jsx component
@hereje hereje force-pushed the refactor/convert-rhs_sidebar-to-ts branch from 1696ffd to cb352e2 Compare October 24, 2024 05:33
@hereje
Copy link
Author

hereje commented Oct 24, 2024

Thanks @hereje. There are some linter issues with this PR. Will you be able to address?

Done!

@hereje hereje requested a review from wiggin77 October 29, 2024 16:26
@wiggin77
Copy link
Member

/update-branch

@mattermost-build
Copy link
Contributor

Error trying to update the PR.
Please do it manually.

@wiggin77
Copy link
Member

@raghavaggarwal2308 can you help get past the CI errors?

@Kshitij-Katiyar
Copy link
Contributor

Kshitij-Katiyar commented Oct 30, 2024

@raghavaggarwal2308 can you help get past the CI errors?

Hey @wiggin77, I investigated the CI failure but couldn't identify any error logs or specific cause for it. It seems unclear where the issue is originating, but it might be related to recent changes in the master branch.
Similar issue is with this Jira PR

@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@Kshitij-Katiyar
Copy link
Contributor

Kshitij-Katiyar commented Nov 18, 2024

@hereje Can you please sync this PR with the latest master? This will fix the failing CI
CC: @wiggin77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Lifecycle/1:stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert rhs_sidebar component to typescript
5 participants