feat/auth-google - #72
Conversation
…icker integration
…hing status details
…nce Picker integration
…r web component and remove deprecated API key requirement
…o reflect completed tasks
|
Why was this closed? It's the whole Google Drive adapter, around 3800 lines, and it's only 2 commits behind main so it would still rebase cleanly. The branch is deleted but the PR ref still has everything: If it was closed because it wasn't finished, fine, but a closed PR with a deleted branch doesn't show up anywhere anyone actually looks. Can we either reopen it or open an issue with what's left? Otherwise we'll write it again in few weeks. |
39ae78f to
20bc2ae
Compare
|
Can we rebase this one? It's ~47 commits behind main and GitHub can't work out whether it merges, so reviewing 3800 lines against a base from two weeks ago isn't going to be worth much. Bigger question before either of us spends time on it: this adds Google sign in, but #111 asks to remove Google sign in because it isn't configured. Which way are we going? Happy to review properly once that's settled and it's rebased. |
|
This is 70 commits behind main now, so I can't review 3800 lines against a July base. The branch is still up, so nothing's at risk either way. Before you spend time rebasing: #111 asks to remove Google sign-in because it isn't configured, and this adds it. I'll settle that first and come back to you, don't touch this one until I do. |
|
Settling this properly, and the short version is that it is blocked on me, not on you. I have been treating #111 and this PR as a choice between Google in or Google out. That was wrong. #111 says the button should go because Google is not configured yet, which is a stopgap for a broken control, not a decision to drop Drive. Drive is still in the product definition. Sorry for leaving that ambiguous for a month, it is the reason this sat. Two corrections on my side. I kept calling this 3800 lines, but 1930 of those are package-lock.json. The real change is 1870 insertions across 19 files and about 460 of that is tests, so it is perfectly reviewable and I should not have used the size as a reason to put it off. The actual blocker is that it needs GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, GOOGLE_PICKER_API_KEY and GOOGLE_CLOUD_PROJECT_NUMBER, and none of those exist because I have not made the Cloud project or enabled the Drive and Picker APIs. Until I do that, nobody can run or deploy this however good the code is. That is my job and I have not started it. There is also a timing problem: this touches storageService.js, ConnectView.jsx and auth.js, and #104 rewrites all three. #104 is approved and about to merge, so rebasing now would just be rework. Marking this as a draft rather than closing it. The work is sound and it is coming back, the state just needs to be honest about it not being reviewable yet. It comes off draft when two things are true: #104 and #120 have merged, and the Google credentials exist. Both are visible so you are not guessing. Nothing for you to do here in the meantime, and do not rebase it. |
|
Closing this properly rather than leaving it as a draft, because the answer changed since this morning. We are dropping Google rather than deferring it. vizably is GitHub only from here, and I am taking the Drive half out of the product definition in CLAUDE.md so nothing keeps promising it. That is a product call about who the tool is for, and it is mine, not a reflection on this branch. To be clear about the work itself: this was not rejected on quality and it did not stall because of anything you did. It stalled because I spent a month treating #111 and this PR as a contradiction when they were not, and because the Google Cloud credentials it needs were always my job and I never started them. The Picker integration and the Drive adapter were the right shape. The branch stays up, so if Drive ever comes back this is the starting point rather than a rewrite. #111 now goes ahead as you originally wrote it, the button comes out. I have added scope notes there, and it wants to land after #104. |
Phase 3: Google — Backend adapter + Picker
One adapter behind the existing provider-neutral interface;