Skip to content

Table configs as protos - #1780

Merged
Ekrekr merged 16 commits into
mainfrom
table-config-as-protos
Jan 17, 2025
Merged

Table configs as protos#1780
Ekrekr merged 16 commits into
mainfrom
table-config-as-protos

Conversation

@Ekrekr

@Ekrekr Ekrekr commented Jul 7, 2024

Copy link
Copy Markdown
Contributor

This removes duplicate javascript interfaces, and duplicate constructors, for the view and incremental table types from the table action.

This means that we're very close to being able to generate a single source of truth for what options are valid for SQLX config blocks, and for javascript interface methods.

It also removes a lot of tech debt, around having to maintain multiple definitions of what are essentially the same options.

@Ekrekr
Ekrekr requested a review from a team as a code owner December 27, 2024 16:42
@Ekrekr
Ekrekr requested review from DiasDauletov and removed request for a team and DiasDauletov December 27, 2024 16:42
@Ekrekr
Ekrekr requested review from a team and DiasDauletov and removed request for a team January 3, 2025 14:51
Comment thread core/actions/incremental_table.ts
Comment thread core/actions/index.ts Outdated
Comment thread core/actions/table.ts
* within SQLX files, or when using a [Contextable](#Contextable) argument with the JS API.
* @hidden
* This maintains backwards compatability with older versions.
* TODO(ekrekr): consider breaking backwards compatability of these in v4.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for all the TODO(erkerkr) probably it is better to have TODO(issue)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are quite a lot of these, I'll make a triage plan and turn them into bugs in one go

Comment thread core/actions/index.ts
Comment thread core/actions/table.ts Outdated
Comment thread core/session.ts
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.

2 participants