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

Review All Dependabot PRs #1641

Closed
Tracked by #1626
ryanfchase opened this issue Dec 13, 2023 · 4 comments
Closed
Tracked by #1626

Review All Dependabot PRs #1641

ryanfchase opened this issue Dec 13, 2023 · 4 comments
Assignees
Labels
Feature: Code Health Make our code more readable, testable, and modular Role: Frontend React front end work Size: 1pt Can be done in 6 hours

Comments

@ryanfchase
Copy link
Member

ryanfchase commented Dec 13, 2023

Overview

We need to review each of the open Dependabot PRs since they are actionable and cluttering our open PRs.

Resources

All Dependabot PRs: https://github.com/hackforla/311-data/pulls/app%2Fdependabot

Potential Next Steps:

  • move forward with all green checkmark PRs (they are generally safe)
  • review all non-green checkmark PRs (we will discuss strategy in the comments before moving out of New Issue Approval)
@ryanfchase ryanfchase mentioned this issue Dec 13, 2023
9 tasks
@ryanfchase ryanfchase changed the title All dependabot PRs Review All Dependabot PRs Dec 13, 2023
@ryanfchase ryanfchase added Size: 1pt Can be done in 6 hours Feature: Documentation Improvements or additions to documentation Feature: Code Health Make our code more readable, testable, and modular Role: Frontend React front end work and removed Feature: Documentation Improvements or additions to documentation labels Dec 13, 2023
@ryanfchase ryanfchase added this to the X - Technical Debt milestone Dec 13, 2023
@Skydodle Skydodle self-assigned this Dec 20, 2023
@Skydodle
Copy link
Member

Skydodle commented Jan 11, 2024

@ryanfchase Upon closer look at the dependabot PRs, I realized that the most recent one was in March 2023, nearly a year ago. Then we decided to turn off dependabot since then, which means all of the exisiting dependabot PRs are nearly a year old or older.

In this case, I'm pretty firmed that it's not a good idea to merge them at all and it's better to just delete these PRs.

  • Reason 1 is that our application structure has changed massively from March 2023, we must have uninstalled or added new or updated some dependencies in a years time. I believe we also switched from fullstack to frontend only during this period (i'm not sure)?? It just doesn't make sense to merge dependency update suggestions from a previous era where our application was in a different state.
  • Reason 2 All of the PRs were requesting to merge into the 'dev' branch, which from my understanding was also the method from previous era that we dont use anymore. Even if we merged these PRs, it would be irrelevant as we only use the 'main' branch nowadays.

Now if we still have dependabot service on right now, it would automatically update these PRs based on the current status of repo, but we haven't had it on. So these PRs are outdated and they were formulated based on the repo at the time of the PR.

My opinion is that the best course of action is to delete all of these dependabot PRs. Then, after launch has been done, at some point we can turn on dependabot again and it would generate dependency update PRs based on the state of application at the time, and we can manually review and merge the PR. We can also add an automation script in our application to have dependabot PRs to automatically run against coverage tests (which we have to write), and we can choose to have them automatically merged if no conflict found from tests (which is common), or we can choose to still manually review.

Please let me know what you think, I will go ahead and delete them unless you think otherwise. Feel free to message me on Slack as well. Thank you!

@ryanfchase
Copy link
Member Author

Thanks @Skydodle, this analysis looks good to me. Tagging @edwinjue for confirmation to close all Dependabot PRs.

@edwinjue
Copy link
Member

@Skydodle Thank you for taking the lead on this! I approve. Please go ahead remove all dependabot-related PRs and as suggested, we can look at reactivating this service after launch.

@Skydodle
Copy link
Member

All dependency updates PRs from dependabot prior to April 2023 are now closed without merge. We will reactivate dependabot and review dependency updates after app launch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Code Health Make our code more readable, testable, and modular Role: Frontend React front end work Size: 1pt Can be done in 6 hours
Projects
Status: Done (without merge)
Development

No branches or pull requests

3 participants