-
Notifications
You must be signed in to change notification settings - Fork 83
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 dead code #317
Remove dead code #317
Conversation
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
33b78c8
to
d2e5605
Compare
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
d2e5605
to
23ca98f
Compare
Signed-off-by: Loan Laux <[email protected]>
Signed-off-by: Loan Laux <[email protected]>
@willopez @chrispotter I can't request reviews from you guys, but feel free to assign yourselves and look through the changes when you get a chance. |
Thanks for the feedback @balibebas. Is it hanging on connection attempts to the DB? Or to the GQL API? Haven't seen that happen on my end, but it's a major issue indeed if it's caused by these changes. |
You're too quick. Right after I wrote that I tried with beta 9 and see it's unrelated to this branch. :D Will let you know when I get it up and running if I can before moving on. |
Tested by building an image as I wasn't able to get the dev link working for this repo. Imported a fresh MongoDB and performed DB migrations. Then registered a user and tested the following:
Didn't encounter any errors and everything looks at parity with the existing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks this much needed clean up! I tested the app and everything seems ok.
Impact: major
Type: performance|refactor|chore
Issue
There's a lot of dead code, mainly comprised of unused components, HOCs and pages.
I know that
reaction-admin
will soon be phased out, but to me it looks like we're still a couple of months away from this happening. I could be wrong, but still, to me it seems that such a clean-up would be welcome to the people and companies who are usingreaction-admin
on a daily basis.Solution
This PR removes all of the dead code I could find. Some may have slipped through the cracks, but I believe this will already be a very welcome clean-up.
My methodology with this PR can be described in a single GIF:
Hence, there's a possibility I could have removed some things that weren't supposed to be removed. I've taken all the precautionary measures I could (test all features, do global searches for usage, etc), but you never know. Testing all kinds of obscure, hidden and forgotten features would be very much appreciated to make sure everything works as it should.
Breaking changes
Not sure but it doesn't appear so. Would appreciate feedback.
Testing