Phase 1 of remove assigned pages #395
Conversation
|
Also forgot to reference the issue/made this a real PR instead of a draft so am doing everything wrong this morning apparently. |
Mr0grog
left a comment
Mr0grog
left a comment
There was a problem hiding this comment.
I think it might be helpful to keep this change a bit simpler and focus only on the first two checkboxes from #394:
- Remove nav links
- Remove all the complex data loading logic around asssigned pages in
src/components/web-monitoring-ui.jsx
And don’t worry so much about the env vars, the server bits, or the API service (i.e. src/services/web-monitoring-api.js and src/services/__tests__/web-monitoring-api.test.js). You can come back and do those in a second PR.
I think I should also remove references to
GOOGLE_TASK_SHEET_ID
You could, but if you want to split up the work as suggested above, just leave it for now.
Sounds good, I think I didn't realize how intensive this was going to be until I started looking into it, so will split it up into multiple PRs as you suggested. |
|
We can also pair on it Wednesday night if you want! |
SYU15
force-pushed
the
rm-assigned-pages
branch
from
cd35b04 to
939ead4
Compare
SYU15
force-pushed
the
rm-assigned-pages
branch
3 times, most recently
from
2d207d1 to
d5d0b09
Compare
SYU15
force-pushed
the
rm-assigned-pages
branch
from
d5d0b09 to
311fd27
Compare
Mr0grog
left a comment
Mr0grog
left a comment
There was a problem hiding this comment.
BURN IT ALL DOWN 🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥
|
(That is to say, looks good, less code, ship it 👍) |
2 participants
I'm at like a 0% confidence level that I killed this correctly, but wanted to throw this up there so I could start getting feedback. I think I should also remove references to
GOOGLE_TASK_SHEET_IDbut wasn't sure so have left it for the time being. Will also do a bunch of spot-checking before I merge this in, of course.