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

feat!: move table to query param & rename existing headers and query params #1900

Merged
merged 11 commits into from
Nov 5, 2024

Conversation

thruflo
Copy link
Contributor

@thruflo thruflo commented Oct 29, 2024

Included renames:

  • electric-chunk-last-offset -> electric-offset
  • electric-next-cursor -> electric-cursor
  • electric-shape-id -> electric-handle
  • ?shape_id -> ?handle

Added query parameter: ?table instead of /:table in the path

Closes #1771, #1796, #1798

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for electric-next ready!

Name Link
🔨 Latest commit 4d92d2b
🔍 Latest deploy log https://app.netlify.com/sites/electric-next/deploys/672a3afd371bf900082f15e8
😎 Deploy Preview https://deploy-preview-1900--electric-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@thruflo thruflo force-pushed the thruflo/header-naming branch from 343b52c to 1d0ed6f Compare October 29, 2024 15:48
@thruflo thruflo marked this pull request as ready for review October 29, 2024 15:50
@thruflo
Copy link
Contributor Author

thruflo commented Oct 31, 2024

Btw, I don't understand why the typecheck tests are failing. pnpm typecheck runs fine for me locally in both the react-hooks and typescript-client packages:

➜  react-hooks git:(thruflo/header-naming) ✗ pnpm run typecheck

> @electric-sql/[email protected] typecheck /Users/thruflo/Development/electric-sql/electric/packages/react-hooks
> tsc -p tsconfig.json

➜  react-hooks git:(thruflo/header-naming) ✗ cd ..
➜  packages git:(thruflo/header-naming) ✗ cd typescript-client
➜  typescript-client git:(thruflo/header-naming) ✗ pnpm run typecheck

> @electric-sql/[email protected] typecheck /Users/thruflo/Development/electric-sql/electric/packages/typescript-client
> tsc -p tsconfig.json

@msfstef
Copy link
Contributor

msfstef commented Oct 31, 2024

@thruflo the react-hooks package depends on the typescript client, so you need to first pnpm build the typescript client and then run pnpm typecheck on the react hooks package

@thruflo
Copy link
Contributor Author

thruflo commented Oct 31, 2024

Yup, I've pnpm -r builded from the repo root locally.

I'm not really the right person to revise the way we run our CI tests :)

@msfstef
Copy link
Contributor

msfstef commented Oct 31, 2024

@thruflo I was also seeing the same issue - typecheck passed locally but failed on CI, and the lines referenced on CI for the errors did not match the actual test file, so something odd happening there...

There's a conflict now so tests aren't running though.

@thruflo thruflo force-pushed the thruflo/header-naming branch from ba5ec88 to 8261b25 Compare October 31, 2024 15:58
@thruflo
Copy link
Contributor Author

thruflo commented Oct 31, 2024

@msfstef fixed the conflict.

@msfstef
Copy link
Contributor

msfstef commented Oct 31, 2024

@thruflo types fixed 💪 🚀

@icehaunter icehaunter force-pushed the thruflo/header-naming branch from 7486f2d to 4d92d2b Compare November 5, 2024 15:34
@icehaunter icehaunter changed the title api: header and parameter names. feat!: move table to query param & rename existing headers and query params Nov 5, 2024
@icehaunter icehaunter merged commit 4d872b6 into main Nov 5, 2024
23 checks passed
@icehaunter icehaunter deleted the thruflo/header-naming branch November 5, 2024 15:46
thruflo added a commit that referenced this pull request Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shape API tweaks
4 participants