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

bindings-macro: add note re. autoinc #59

Merged
merged 4 commits into from
Jul 20, 2023
Merged

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jul 5, 2023

Description of Changes

Adds documentation on #[autoinc] not implying #[unique] / #[primarykey] per discussion on Discord.

API

  • This is a breaking change to the module API
  • This is a breaking change to the ClientAPI

If the API is breaking, please state below what will break

@Centril Centril requested review from jdetter and gefjon July 5, 2023 17:09
Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

It looks a bit odd having those notes, but nothing else about the grammar or semantics of the macro. Didn't we have a ticket for documenting the macro?

Centril and others added 2 commits July 6, 2023 17:27
Co-authored-by: Kim Altintop <kim@eagain.io>
Signed-off-by: Mazdak Farrokhzad <twingoow@gmail.com>
@Centril Centril requested review from kim and gefjon July 6, 2023 15:32
crates/bindings-macro/src/lib.rs Outdated Show resolved Hide resolved
crates/bindings-macro/src/lib.rs Show resolved Hide resolved
Copy link
Contributor

@kim kim left a comment

Choose a reason for hiding this comment

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

For me this works.

I would intuitively translate those attributes to SQL, so questions of which can / should be combined didn't arise. Maybe we can draw that analogy more often, as we seem to default to SQL semantics airquotes-unless-otherwise-noted.

@Centril Centril requested a review from gefjon July 20, 2023 15:22
@Centril Centril merged commit caa4d61 into master Jul 20, 2023
4 checks passed
cloutiertyler pushed a commit that referenced this pull request Aug 1, 2023
* bindings-macro: add note re. autoinc

* Update crates/bindings-macro/src/lib.rs

Co-authored-by: Kim Altintop <kim@eagain.io>
Signed-off-by: Mazdak Farrokhzad <twingoow@gmail.com>

* spacetimedb_tabletype: minor docs tweaks

* address pheobe's review

---------

Signed-off-by: Mazdak Farrokhzad <twingoow@gmail.com>
Co-authored-by: Kim Altintop <kim@eagain.io>
cloutiertyler pushed a commit that referenced this pull request Aug 1, 2023
* bindings-macro: add note re. autoinc

* Update crates/bindings-macro/src/lib.rs

Co-authored-by: Kim Altintop <kim@eagain.io>
Signed-off-by: Mazdak Farrokhzad <twingoow@gmail.com>

* spacetimedb_tabletype: minor docs tweaks

* address pheobe's review

---------

Signed-off-by: Mazdak Farrokhzad <twingoow@gmail.com>
Co-authored-by: Kim Altintop <kim@eagain.io>
@cloutiertyler cloutiertyler deleted the centril/note-autoinc branch August 1, 2023 21:53
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