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

Implement rank_user_replies in web #127

Merged
merged 2 commits into from
Dec 29, 2022
Merged

Implement rank_user_replies in web #127

merged 2 commits into from
Dec 29, 2022

Conversation

AbdBarho
Copy link
Collaborator

Closes #33

Also started moving duplicate code into their own components

@AbdBarho AbdBarho marked this pull request as draft December 28, 2022 22:05
@fozziethebeat
Copy link
Collaborator

I see you marked this as draft. Will approve once you set it to ready for review again.

@AbdBarho
Copy link
Collaborator Author

AbdBarho commented Dec 29, 2022 •
edited
Loading

I see you marked this as draft. Will approve once you set it to ready for review again.

I don't want to unset draft because I am not able to test against our backend.

Sampling random task of any kinds returns the error No Threads Found #140,
I think the problem is that in case there are no threads, no new dummy ones are generated...

I will see if I can fix this

EDIT: it is fixed now, ready for review!

@yk
Copy link
Collaborator

yk commented Dec 29, 2022

this will be fixed by #144 , sorry for the inconvenience

@AbdBarho AbdBarho force-pushed the rank_user_replies branch 2 times, most recently from 099d3d8 to fd1311b Compare December 29, 2022 15:51
@AbdBarho AbdBarho marked this pull request as ready for review December 29, 2022 15:53
@AbdBarho AbdBarho mentioned this pull request Dec 29, 2022
Merged
@AbdBarho AbdBarho enabled auto-merge December 29, 2022 19:43
k-nearest-neighbor
k-nearest-neighbor reviewed Dec 29, 2022
View reviewed changes
website/src/components/Sortable/Sortable.tsx
))}
</ul>
);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking to merge this in just to get the template and the bulk of the work so far in to the repo 🥳
But I think we should be able to replace with https://dndkit.com/ for the MVP

k-nearest-neighbor
k-nearest-neighbor approved these changes Dec 29, 2022
View reviewed changes
Copy link
Collaborator

@k-nearest-neighbor k-nearest-neighbor left a comment

Choose a reason for hiding this comment

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

Excellent. Ok I think there are 2 areas still to do with rank_user_replies:

  • Drag and drop with https://dndkit.com/
  • Some adjustments to the task description and possibly design; As a user I do not understand this task (what are the replies replying to?)

Approving this to merge in the progress

@AbdBarho AbdBarho mentioned this pull request Dec 29, 2022
Closed
andreaskoepf
andreaskoepf approved these changes Dec 29, 2022
View reviewed changes
Copy link
Collaborator

@andreaskoepf andreaskoepf left a comment

Choose a reason for hiding this comment

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

trusting knn. :-)

@AbdBarho AbdBarho merged commit 2990bb9 into main Dec 29, 2022
@AbdBarho AbdBarho deleted the rank_user_replies branch December 29, 2022 20:06
@AbdBarho AbdBarho mentioned this pull request Dec 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreaskoepf andreaskoepf andreaskoepf approved these changes

@k-nearest-neighbor k-nearest-neighbor k-nearest-neighbor approved these changes

@fozziethebeat fozziethebeat Awaiting requested review from fozziethebeat

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

Assignees
No one assigned
Labels
website
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement rank_user_replies in web
5 participants
@AbdBarho @fozziethebeat @yk @andreaskoepf @k-nearest-neighbor