routes.py and test_routes.py are HUGE
Closed this issue · 5 comments
In the past, it didn't bother me that routes.py and test_routes.py were both over 250 lines of code. However, we've gotten to the point where routes.py is close to 500 LOC and test_routes.py is almost 1000 (and after #244 is merged, it will be over 1000). It's time to go ahead and break these up into smaller files so we can more easily find what we're looking for.
I'm open to how it should be organized, no strong opinions yet. If I were to do it myself, I'd probably group it similar to the documentation where each section was in it's own file. So you'd have search_routes.py, resources_routes.py, categories_routes.py and languages_routes.py. Alternatively, there could be new directories for each of these and a routes.py in each of them, though I don't know if that makes a lot of sense because I imagine that routes.py is all that would go in those directories. The test files would be broken up similarly, though depending on how big each file is, they may need to be broken down further, but we won't know until we go in there and break them up and see what happens. I would hope that each would be less than 250 LOC but I honestly don't know.
@aaron-suarez; I'm willing to start tackling this. Would you mind if I do this on a separate PR per organizational boundary? I'm thinking of creating a routes package in place of the routes module, and then having a single module per functional area. The first PR would be groundwork preparing for this, and then the subsequent PRs would be for each functional area and all would be based on the preparatory PR. This should make it easier to review each PR, and, honestly, give me some Hacktoberfest cred 😉
Actually, I think I will have a base branch with the setup, and just do a separate PR for each boundary, instead of having a separate setup PR which probably wouldn't be that big anyways.
Annndddd, I'm realizing that it's going to be a mess splitting them up into separate PRs. Instead, I think I'll have one PR for routes cleanup and another for route test cleanup. Otherwise, it's probably going to be more confusing to actually go through the PRs, as they will all be dependent on each other (due to modifying the same file over and over again.)
Yeah, I'd much prefer it to be one PR for routes and another PR for the tests. That way, we can verify that moving the routes doesn't break the tests (so do that one first). Then do the tests as a follow up so we can break it down but we can still have confidence that everything is still working fine.
Yeah, that's my current plan. Just waiting for feedback before tackling the merge conflicts.