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.

Sign up for GitHub

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

Jump to bottom

Convert files related to redux from js to ts #462

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

Convert files related to redux from js to ts #462

wants to merge 24 commits into from

Conversation

raghavaggarwal2308
Copy link
Contributor

Summary

  • Convert files related to redux from js to ts
  • Removed some unused code
  • Fixed CSS warnings

What to test?

  • Complete testing of the plugin.

@codecov Codecov
Copy link

codecov bot commented Feb 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.02%. Comparing base (191a11a) to head (7eaf225).
Report is 7 commits behind head on master.

❗ Current head 7eaf225 differs from pull request most recent head 98b9f75

Please upload reports for the commit 98b9f75 to get more accurate results.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #462   +/-   ##
=======================================
  Coverage   34.02%   34.02%           
=======================================
  Files          22       22           
  Lines        4021     4021           
=======================================
  Hits         1368     1368           
  Misses       2515     2515           
  Partials      138      138

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mickmister mickmister mentioned this pull request Mar 11, 2024
Open
mickmister
mickmister reviewed Mar 14, 2024
View reviewed changes
Copy link
Member

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Nice work @raghavaggarwal2308 👍 Just a few comments on adding some more types

webapp/src/actions/index.ts Show resolved Hide resolved
webapp/src/components/rhs_sidebar/rhs_sidebar.css Show resolved Hide resolved
webapp/src/reducers/index.ts Outdated Show resolved Hide resolved
webapp/src/types/store.ts Outdated Show resolved Hide resolved
webapp/src/selectors/index.ts Outdated Show resolved Hide resolved
webapp/src/selectors/index.ts Outdated Show resolved Hide resolved
@ayusht2810
Copy link
Contributor

@mickmister Fixed the review comments. Please re-review.

mickmister
mickmister reviewed Mar 27, 2024
View reviewed changes
Copy link
Member

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Amazing work 👍 I have just one last request to convert a small component's index.ts and component file to test drive how this all works. Great job!

webapp/src/types/store.ts Outdated Show resolved Hide resolved
webapp/src/selectors/index.ts Outdated Show resolved Hide resolved
webapp/src/types/store.ts Outdated Show resolved Hide resolved
@ayusht2810 ayusht2810 linked an issue Mar 28, 2024 that may be closed by this pull request
Convert user_attribute component to typescript #434
Open
@ayusht2810
Copy link
Contributor

@mickmister Gentle reminder for the re-review

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Apr 19, 2024
@hanzei hanzei requested a review from mickmister April 19, 2024 08:46
mickmister
mickmister reviewed May 14, 2024
View reviewed changes
Copy link
Member

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Mostly looks good, just a few more suggestions and questions

webapp/src/actions/index.ts Outdated Show resolved Hide resolved
webapp/src/components/user_attribute/user_attribute.tsx Outdated Show resolved Hide resolved
webapp/src/components/user_attribute/index.ts Outdated Show resolved Hide resolved
webapp/src/components/user_attribute/user_attribute.tsx Outdated Show resolved Hide resolved
webapp/src/components/user_attribute/index.ts Show resolved Hide resolved
@raghavaggarwal2308
Copy link
Contributor Author

@mickmister Fixed the review comments added by you

@raghavaggarwal2308 raghavaggarwal2308 added this to the v1.10.0 milestone Jul 1, 2024
@Kshitij-Katiyar
Copy link
Contributor

@mickmister Implemented the suggestions. Please have a look

mickmister
mickmister reviewed Jul 10, 2024
View reviewed changes
webapp/src/client/client.ts Outdated Show resolved Hide resolved
webapp/src/client/client.ts Outdated Show resolved Hide resolved
webapp/src/client/client.ts Outdated Show resolved Hide resolved
@Kshitij-Katiyar
Copy link
Contributor

Kshitij-Katiyar commented Jul 10, 2024 •
edited by raghavaggarwal2308
Loading

@mickmister @wiggin77 Updated the suggestions, please have a look.

@raghavaggarwal2308 raghavaggarwal2308 self-assigned this Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spirosoik spirosoik Awaiting requested review from spirosoik spirosoik is a code owner

@wiggin77 wiggin77 Awaiting requested review from wiggin77 wiggin77 is a code owner

@mickmister mickmister Awaiting requested review from mickmister

At least 1 approving review is required to merge this pull request.

Assignees

@raghavaggarwal2308 raghavaggarwal2308

Labels
2: Dev Review Requires review by a core committer
Projects
None yet
Milestone
v1.10.0
Development

Successfully merging this pull request may close these issues.

Convert user_attribute component to typescript
5 participants
@raghavaggarwal2308 @ayusht2810 @Kshitij-Katiyar @mickmister @hanzei