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

Conditionally compiled PoT support #1796

Merged
merged 1 commit into from
Aug 11, 2023
Merged

Conversation

nazar-pc
Copy link
Member

This by default doesn't compile PoT support in a few places (it is still largely in place, but shouldn't break compatibility with devnet), can be enabled like this:

cargo clippy --all-targets --features subspace-node/pot,sp-lightclient/pot,subspace-test-service/pot

Code contributor checklist:

rahulksnv
rahulksnv previously approved these changes Aug 10, 2023
Copy link
Contributor

@rahulksnv rahulksnv left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this, LGTM.

Thought for future: standard way of handling these scenarios is use protobufs (or equivalent) where the upgraded code can handle absence of fields. Guess that is also possible with SCALE, we will need some refactoring/versioning

@nazar-pc
Copy link
Member Author

Forgot to commit changes in one of the files.

Thought for future: standard way of handling these scenarios is use protobufs (or equivalent) where the upgraded code can handle absence of fields.

I dislike protobuf for a similar reasons I dislike Golang. For instance having all fields optional is not something that I think is a good thing.

Guess that is also possible with SCALE, we will need some refactoring/versioning

You certainly can if you use an enum. Just add a new variant any time to it.

@nazar-pc nazar-pc merged commit 6b7ed31 into main Aug 11, 2023
9 checks passed
@nazar-pc nazar-pc deleted the conditionally-compiled-pot-support branch August 11, 2023 07:40
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