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

api: header and parameter names. #1900

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

api: header and parameter names. #1900

wants to merge 29 commits into from

Conversation

thruflo
Copy link
Contributor

@thruflo thruflo commented Oct 29, 2024

This is a rebase of some changes to rename:

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

Painfully rebased on the previous shape_handle and table path -> query param change in #1798).

So I believe we can merge this instead of those and this will match the new documentation. I haven't yet added a changeset.

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for electric-next ready!

Name Link
🔨 Latest commit 8261b25
🔍 Latest deploy log https://app.netlify.com/sites/electric-next/deploys/6723a9346815ef00084adac4
😎 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 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/react@0.4.7 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/client@0.6.5 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
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 💪 🚀

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.

3 participants