-
Notifications
You must be signed in to change notification settings - Fork 61
Conversation
❌ Deploy Preview for ubiquibot-staging failed.
|
The merge-base changed after approval.
should I resolve the conflicts? @pavlovcik |
This should have merged this in weeks ago. The number of outstanding pull requests is growing every month and this is unacceptable. Most of these code changes in this pull request were simple renaming of variables. It would have taken 15 minutes to review and approve @0xcodercrane
I did a pass at it because I know the code I wrote, and I've also been looking at all the pull requests so I have sufficient context. I did it quickly in the GitHub UI though. Please ensure that it builds etc. |
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.
We shouldn't delete yarn.lock because it's being used in github ci / netlify ci. Due to permission, we couldn't generate it on both platform. That's why we've been keeping it from the beginning.
@0xcodercrane In this case you should consider just regenerating the lock file and merging instead of kicking the can down the road. I'm not at home so I don't have the capability to from my phone. |
should I merge #643 in prior to this PR? @pavlovcik |
Don't think it matters the order regarding conflicts but technically speaking this is branched off of #643 head. |
Will need to e2e test and merge |
Review comments at #849 |
feat: remove axios call
…-call Revert "feat: remove axios call"
@gitcoindev maybe you can help fix the knip CI here |
🎉 |
Resolves #574
Refactoring to clean up the codebase.
Based on #643
Secrets
PROJECT_ID
->SUPABASE_PROJECT_ID
I'm not great at the database side. I realized I changed some property names and might have broken the permits database at 03466d6
Included most of @whilefoo's end to end tests but a few dont work like
allow
andstart/stop
those are commented out