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.

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

Remove unused dependencies #3553

Merged
merged 6 commits into from
Oct 24, 2024
Merged

Conversation

tdonohue
Copy link
Member

Description

This PR removes a number of dependencies defined in our package.json which are unused by our codebase. Some of these are optional dev tools. But, in my opinion, they can be installed by developers who need them, since DSpace doesn't use them in our code.

One dependency moved from production to dev dependency:

  • @types/grecaptcha - this is only needed for compilation

Unused production dependencies removed:

  • angular-idle-preload

Unused development dependencies removed:

  • browser-sync
  • react and react-dom
  • rxjs-spy
  • webpack-bundle-analyzer

Instructions for Reviewers

  • Verify build succeeds and all tests pass (especially since most of the removed dependencies are in the dev environment)
  • Verify no adverse behavior to User Interface. I did some basic testing today and didn't notice anything.

@tdonohue tdonohue added dependencies Pull requests that update a dependency file 1 APPROVAL pull request only requires a single approval to merge labels Oct 23, 2024
@tdonohue tdonohue added this to the 9.0 milestone Oct 23, 2024
@tdonohue tdonohue added port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Oct 23, 2024
Copy link
Member

@kshepherd kshepherd left a comment

Choose a reason for hiding this comment

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

+1 tested successfully with dspace theme, all looks good to me!

Copy link

Hi @tdonohue,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

@kshepherd
Copy link
Member

@tdonohue i merged the postcss change which then introduced this merge conflict, whoops!

@tdonohue
Copy link
Member Author

@kshepherd : No worries! I'll get this cleaned up later today and will go ahead and merge it, since both you and I have tested it.

I also have to create backports of this work for 8.x and 7.x (which I didn't find time for yesterday). Thanks for the quick review/test!

@tdonohue
Copy link
Member Author

Rebased this on latest main to solve the merge conflicts. The conflicts were really tiny (one line in package.json and package-lock.json and quick to cleanup

@kshepherd kshepherd merged commit 43630bb into DSpace:main Oct 24, 2024
13 checks passed
@dspace-bot
Copy link
Contributor

Backport failed for dspace-7_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-7_x
git worktree add -d .worktree/backport-3553-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-3553-to-dspace-7_x
git switch --create backport-3553-to-dspace-7_x
git cherry-pick -x 217b72a3329329f5a8c83c4bb2c57982bd165b33 fef5dd72b30ca6174587f1a7dea8994f1e96bbc9 426a0ac4880dbfb2381685ab39222ce347e2bc1c 4c5b064cfb74c9aac263f405248c889e13c81bfe 3b00cc10809fdf801213066bfe64f6dde1996598 3832851ef4295924e079d3ee196ce455d21790eb

@dspace-bot
Copy link
Contributor

Backport failed for dspace-8_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-8_x
git worktree add -d .worktree/backport-3553-to-dspace-8_x origin/dspace-8_x
cd .worktree/backport-3553-to-dspace-8_x
git switch --create backport-3553-to-dspace-8_x
git cherry-pick -x 217b72a3329329f5a8c83c4bb2c57982bd165b33 fef5dd72b30ca6174587f1a7dea8994f1e96bbc9 426a0ac4880dbfb2381685ab39222ce347e2bc1c 4c5b064cfb74c9aac263f405248c889e13c81bfe 3b00cc10809fdf801213066bfe64f6dde1996598 3832851ef4295924e079d3ee196ce455d21790eb

@tdonohue
Copy link
Member Author

I'll manually backport this to both 8.x & 7.x

@tdonohue
Copy link
Member Author

Ported to 8.x in #3573
Ported to 7.x in #3574

These ports are not entirely identical because each version is a bit unique. Some versions use a few of these dependencies. The concept is the same though.

@tdonohue tdonohue removed port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge dependencies Pull requests that update a dependency file
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants