Skip to content
This repository has been archived by the owner on Oct 8, 2023. It is now read-only.

Audit all dependencies #267

Draft
wants to merge 3 commits into
base: staging
Choose a base branch
from
Draft

Audit all dependencies #267

wants to merge 3 commits into from

Conversation

RCode-Blue
Copy link
Collaborator

Overview of the Pull Request:

Ran depcheck to discover missing and unused libraries.

  • Removed react-icons.

link to Trello card or Github issue

https://trello.com/c/BTqLfTNb/75-dev-audit-of-all-dependencies

How Has This Been Tested?

  • Please describe the tests that you ran to verify your changes.
  • Provide instructions so we can reproduce.
  • If necessary, please also list any relevant details for your test setup/configuration

Pull Request Checklist:

Make sure the following checkboxes[ ] are checked with x before sending your PR -- thank you!

  • Have you included a link Trello card / Github issue and written sufficient description to help others review your work?
  • Is your pull request up-to-date with main branch?
  • Have you checked That code is well formatted? Did npx prettier --check . return no warnings/errors?
  • Have you written instructions on how to test that your changes work as expected?
  • Have you tested your work thoroughly on your local machine to ensure you are not breaking anything?

@vercel
Copy link

vercel bot commented Apr 9, 2023

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

Name Status Preview Comments Updated (UTC)
council-emissions-calculator-spike ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 10, 2023 10:38am

Copy link
Collaborator

@OlaShabalina OlaShabalina left a comment

Choose a reason for hiding this comment

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

Hi @RCode-Blue, I can see the trello card is in for review section but PR is a draft. Is PR ready for review?

Copy link
Collaborator

@hqtan hqtan left a comment

Choose a reason for hiding this comment

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

Hi @RCode-Blue ,

Thx for your PR 🙇🏽

Apologies it's taken me so long to review it!
I've left some comments in the PR for you.

@@ -11,6 +11,7 @@
},
"dependencies": {
"@chakra-ui/react": "^1.6.8",
"@chakra-ui/theme-tools": "^2.0.16",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @RCode-Blue ,

why do we need @chakra-ui/theme-tools package?
This dependency wasn't explicitly mentioned in the main branch.

@@ -22,8 +23,7 @@
"highcharts-react-official": "^3.0.0",
"next": "^11.1.0",
"react": "17.0.2",
"react-dom": "17.0.2",
"react-icons": "^4.3.1"
"react-dom": "17.0.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @RCode-Blue ,

we use "react-dom": "^18.2.0" in main branch;
Why should we downgrade the package to version 17.0.2 ?

also, we are using React v18 in main branch. Is there a reason why we should downgrade to v17?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants