Skip to content

Conversation

@Kshitij-Katiyar
Copy link
Contributor

@Kshitij-Katiyar Kshitij-Katiyar commented Nov 19, 2024

Summary

Converting user_attribute component and client files to TS and other changes"
Upgrade mattermost-redux to 10.8.0

Actual PR

#785

@Kshitij-Katiyar Kshitij-Katiyar added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Nov 19, 2024
@Kshitij-Katiyar Kshitij-Katiyar self-assigned this Nov 19, 2024
@wiggin77
Copy link
Member

@Kshitij-Katiyar @raghavaggarwal2308 it's tough to know if something was missed in a PR like this when no unit tests seem to exist that cover the changes. I'm thinking we should hold off on TS conversions until we have unit test coverage. Thoughts?

@raghavaggarwal2308
Copy link

@wiggin77 We are just adding types to the existing files and not adding any functionality, so I am not sure how unit tests will be helpful here.
If you are concerned that something is changed by mistake, we can have a QA do a high level sanity for these PRs.
Please let me know your opinion.

@raghavaggarwal2308 raghavaggarwal2308 removed the 2: Dev Review Requires review by a core committer label Dec 22, 2024
Base automatically changed from MM-944 to master January 9, 2025 11:48
@mm-prodsec-bot
Copy link
Contributor

mm-prodsec-bot commented Jul 3, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@abbas-dependable-naqvi abbas-dependable-naqvi changed the title [MM-945]: Converting user_attribute component and client files to TS [MM-945]: Converting user_attribute component and client files to TS and Upgrade mattermost-redux to 10.8.0 Jul 3, 2025
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.

Build is failing.

@raghavaggarwal2308
Copy link

@wiggin77 FIxed the CI

@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 is this up to date with master?

@raghavaggarwal2308
Copy link

@wiggin77 Yes, its up to date

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.

LGTM 👍

@Yash-Chakerverti Yash-Chakerverti self-requested a review September 4, 2025 13:16
Copy link

@Yash-Chakerverti Yash-Chakerverti left a comment

Choose a reason for hiding this comment

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

This PR is tested & all the code changes are working fine.
LGTM

@abbas-dependable-naqvi abbas-dependable-naqvi merged commit 32b06d2 into master Sep 4, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3: QA Review Requires review by a QA tester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants