Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

global: Use reserved discriminant spaces for TypeId #271

Merged
merged 1 commit into from
May 2, 2022

Conversation

tilacog
Copy link
Contributor

@tilacog tilacog commented Apr 28, 2022

This rearranges TypeId variants to match graph-node's IndexForAscTypeId numbering.

This rearranges TypeId variants to match graph-node's
IndexForAscTypeId numbering.
@tilacog
Copy link
Contributor Author

tilacog commented Apr 28, 2022

@evaporei I'm not sure, but I think this change would require a new pre-release version for graph-ts?
If so, then I believe we should also patch graph-cli to use this latest version, as well.

@tilacog tilacog requested a review from evaporei April 28, 2022 12:16
@evaporei
Copy link
Contributor

@tilacog great comments 👏 ❤️

I just have a quick question, what about new Ethereum and regular types (non-attached to any chain)? Should those stick within the NEAR range?

Thanks a lot for doing this 😊

@tilacog
Copy link
Contributor Author

tilacog commented Apr 28, 2022

I don't think we have support for blockchain agnostic types, but new Ethereum types have their own range here.

We couldn't place them next to the original Ethereum types, but there is room for them to grow over time.

@evaporei
Copy link
Contributor

I don't think we have support for blockchain agnostic types

I mean types like String, ArrayBuffer, etc.

@tilacog
Copy link
Contributor Author

tilacog commented Apr 28, 2022

Hm, I see.

Since they were placed next to Ethereum types, originally, I think we can continue placing them as so.
We still have ~500 slots reserved for that. If that is not enough, we can reserve more slots at the end.

Do you think that would work?

@tilacog
Copy link
Contributor Author

tilacog commented Apr 29, 2022

@evaporei It seems CI is not running for this, so I can't merge it.
I can't see any associated checks for this PR, though.

@tilacog tilacog force-pushed the tiago/type-id-reserved-discriminant-space branch from f76ae91 to 07160b4 Compare May 2, 2022 21:54
@evaporei evaporei merged commit 5368042 into main May 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants