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

Added multi column index usage to the rust-wasm-test module #500

Merged
merged 4 commits into from
Nov 7, 2023

Conversation

cloutiertyler
Copy link
Contributor

Description of Changes

Just adds usage of the multi column index macro feature

API and ABI

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

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

Expected complexity level and risk

How complicated do you think these changes are? Grade on a scale from 1 to 5,
where 1 is a trivial change, and 5 is a deep-reaching and complex change.

This complexity rating applies not only to the complexity apparent in the diff,
but also to its interactions with existing and future code.

If you answered more than a 2, explain what is complex about the PR,
and what other components it interacts with in potentially concerning ways.

`

@kulakowski
Copy link
Contributor

Did this tickle the very brittle standalone integration test? It has a comment about needing to run things in order, and the failing assertion is that the number of lines in some log output is off.

@kim
Copy link
Contributor

kim commented Oct 31, 2023

Yes I think the test is failing because creating the index writes to the module log.

Copy link
Contributor

@kulakowski kulakowski left a comment

Choose a reason for hiding this comment

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

The introduction of the Point table itself seems fine, so approving, because the bug fix seems trivial and unrelated.

The number of lines in the output changes as there is 1 additional
line corresponding to the new table.
@kulakowski kulakowski enabled auto-merge (squash) November 6, 2023 21:09
@kulakowski kulakowski merged commit 8ba8dba into master Nov 7, 2023
kulakowski pushed a commit that referenced this pull request Nov 7, 2023
Added multi column index usage to the rust-wasm-test module

This also updates integration test.

The number of lines in the output changes as there is 1 additional
line corresponding to the new table.
kulakowski pushed a commit that referenced this pull request Nov 7, 2023
Added multi column index usage to the rust-wasm-test module

This also updates integration test.

The number of lines in the output changes as there is 1 additional
line corresponding to the new table.
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