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

feat: deleting brains on brain manager page #893

Merged
merged 4 commits into from
Aug 8, 2023
Merged

feat: deleting brains on brain manager page #893

merged 4 commits into from
Aug 8, 2023

Conversation

StepanLebedevTheodo
Copy link
Contributor

No description provided.

@StepanLebedevTheodo StepanLebedevTheodo temporarily deployed to preview August 8, 2023 14:52 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Aug 8, 2023 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 8, 2023 4:15pm
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 8, 2023 4:15pm

@StepanLebedevTheodo StepanLebedevTheodo mentioned this pull request Aug 8, 2023
Closed
@StepanLebedevTheodo StepanLebedevTheodo changed the title Feat: deleting brains on brain manager page feat: deleting brains on brain manager page Aug 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023 •
edited

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/lib/context/BrainProvider/hooks/useBrainProvider.ts

The code seems to be well written and follows the SOLID principles. However, there is a potential issue with the useBrainProvider hook. It seems to be handling too many responsibilities, such as managing brains, handling deletion, and managing modals. Consider breaking it down into smaller, more manageable hooks to improve readability and maintainability.

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/BrainManagementTabs.tsx

The code seems to be well written and follows the SOLID principles. However, there is a potential issue with the BrainManagementTabs component. It seems to be handling too many responsibilities, such as managing tabs, handling deletion, and managing modals. Consider breaking it down into smaller, more manageable components to improve readability and maintainability.

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/hooks/useBrainManagementTabs.ts

The code seems to be well written and follows the SOLID principles. However, there is a potential issue with the useBrainManagementTabs hook. It seems to be handling too many responsibilities, such as managing tabs, handling deletion, and managing modals. Consider breaking it down into smaller, more manageable hooks to improve readability and maintainability.

📚🔧🔀

Powered by Code Review GPT

mamadoudicko
mamadoudicko reviewed Aug 8, 2023
View reviewed changes
Copy link
Contributor

@mamadoudicko mamadoudicko left a comment

Choose a reason for hiding this comment

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

Nice Pr @StepanLebedevTheodo! Well done

@StepanLebedevTheodo StepanLebedevTheodo temporarily deployed to preview August 8, 2023 15:53 — with GitHub Actions Inactive
@mamadoudicko mamadoudicko force-pushed the feat/deleting-brains-on-brain-page branch from 6cd88f0 to 6794e64 Compare August 8, 2023 16:05
@mamadoudicko mamadoudicko temporarily deployed to preview August 8, 2023 16:05 — with GitHub Actions Inactive
@mamadoudicko mamadoudicko force-pushed the feat/deleting-brains-on-brain-page branch from 6794e64 to 0e522da Compare August 8, 2023 16:12
@mamadoudicko mamadoudicko temporarily deployed to preview August 8, 2023 16:12 — with GitHub Actions Inactive
@StanGirard StanGirard merged commit 71e142b into main Aug 8, 2023
7 checks passed
@StanGirard StanGirard mentioned this pull request Aug 8, 2023
Merged
@StepanLebedevTheodo StepanLebedevTheodo deleted the feat/deleting-brains-on-brain-page branch August 8, 2023 20:08
StanGirard pushed a commit that referenced this pull request Sep 12, 2023
@StepanLebedevTheodo
feat: deleting brains on brain manager page (#893)
a17bfae 
* fix: new libraries for translation

* feat(deleting-brains-on-brain-management-page): adding a delete brain button and a confirmation modal on a management page

* feat(deleting-brains-on-brain-management-page): adding tests for confirmation Modal

* feat(deleting-brains-on-brain-management-page): Adding translation to the delete button and modal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mamadoudicko mamadoudicko mamadoudicko approved these changes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@StepanLebedevTheodo @mamadoudicko @StanGirard