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 LoadingArchiveComponent #379

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

meisekimiu
Copy link
Member

The LoadingArchiveComponent was created to be an interstitial component displayed when switching archives in commit 76883bd. However, as time has gone on, the /app/switching path became unused. Since this specific LoadingArchiveComponent was only used in this one route, the component itself became unused. Presumably it stayed around because it sounds important!

This component has problematic tests. They are (relatively) slow since they wait 2.5 seconds just for one test, and also sometimes fail due to random asynchronous errors. While we could optimize the component to fix these errors, since the component is unused we should just delete it and the problematic tests.

Steps to test:

  1. Run tests/build/etc.
  2. Verify that nothing breaks in the web-app when loading things, especially when switching archives.

Resolves PER-9520.

@meisekimiu meisekimiu requested a review from k8lyn6 April 1, 2024 22:27
@meisekimiu
Copy link
Member Author

Requesting QA on this one just to be safe. In theory there are no user-facing changes but perhaps this breaks something I didn't see? For QA one should mainly check that you can still switch archives.

Copy link

@k8lyn6 k8lyn6 left a comment

Choose a reason for hiding this comment

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

Switching and creating archives on both new and old accounts seems to work just fine!

The LoadingArchiveComponent was created to be an interstitial component
displayed when switching archives in commit 76883bd. However, as time
has gone on, the `/app/switching` path became unused. Since this
specific LoadingArchiveComponent was only used in this one route, the
component itself became unused.

This component has problematic tests. They are (relatively) slow since
they wait 2.5 seconds just for one test, and also sometimes fail due to
random asyncronous errors. While we could optimize the component to fix
these errors, since the component is unused we should just delete it and
the problematic tests.

Resolves PER-9520
@meisekimiu meisekimiu force-pushed the remove-loading-archive-component branch from 06a4a7a to 5292b8d Compare April 2, 2024 19:25
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.75%. Comparing base (27383ac) to head (5292b8d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #379   +/-   ##
=======================================
  Coverage   36.74%   36.75%           
=======================================
  Files         308      307    -1     
  Lines       10615    10611    -4     
  Branches     1763     1763           
=======================================
- Hits         3901     3900    -1     
+ Misses       6572     6569    -3     
  Partials      142      142           

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

@meisekimiu meisekimiu merged commit b7d4e44 into main Apr 2, 2024
2 checks passed
@meisekimiu meisekimiu deleted the remove-loading-archive-component branch April 2, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants