Skip to content

feat: use new IterableMap and IterableSet collections#139

Closed
aleksuss wants to merge 7 commits into
near:masterfrom
aleksuss:feat/new_collections
Closed

feat: use new IterableMap and IterableSet collections#139
aleksuss wants to merge 7 commits into
near:masterfrom
aleksuss:feat/new_collections

Conversation

@aleksuss
Copy link
Copy Markdown
Collaborator

@aleksuss aleksuss commented Jul 15, 2024

TODO: Check backward compatibility.

@near near deleted a comment Aug 17, 2024
@near near deleted a comment Aug 17, 2024
@near near deleted a comment Aug 17, 2024
@karim-en
Copy link
Copy Markdown
Collaborator

@aleksuss Looks like the changes are backward compatible. We have added a test here, please merge with master to check it out.

@aleksuss aleksuss closed this Aug 26, 2024
@aleksuss aleksuss reopened this Aug 26, 2024
@aleksuss
Copy link
Copy Markdown
Collaborator Author

Looks like the changes are backward compatible.

@karim-en But the test says the opposite.

@karim-en
Copy link
Copy Markdown
Collaborator

karim-en commented Aug 26, 2024

Looks like the changes are backward compatible.

@karim-en But the test says the opposite.

Sorry it was a typo. I wanted to say "aren't backward compatible"

@aleksuss
Copy link
Copy Markdown
Collaborator Author

I thought to add these changes to the 0.3.0 version, but as I can see, this version has already been released.

@karim-en
Copy link
Copy Markdown
Collaborator

I thought to add these changes to the 0.3.0 version, but as I can see, this version has already been released.

It is dangerous to do a release like this because the plugin's users would like to upgrade to the latest version and miss the plugins being upgradable and needing a migration.
What I can propose is adding a non-default feature flag for these changes.

@aleksuss
Copy link
Copy Markdown
Collaborator Author

Changing a minor version if the major is 0 implies the existence of breaking changes by semver rules. As for me, I don't like a feature flag. I would postpone these changes to the next release though.

@karim-en
Copy link
Copy Markdown
Collaborator

karim-en commented Aug 26, 2024

Changing a minor version if the major is 0 implies the existence of breaking changes by semver rules. As for me, I don't like a feature flag. I would postpone these changes to the next release though.

It is storage breaking changes, we can't allow releasing things like this without proper storage migration implementation.
The breaking changes are acceptable just on the code or API level.

Check these contracts
https://github.com/near/near-sdk-rs/tree/master/near-contract-standards

@karim-en
Copy link
Copy Markdown
Collaborator

karim-en commented Aug 26, 2024

Changing a minor version if the major is 0 implies the existence of breaking changes by semver rules. As for me, I don't like a feature flag. I would postpone these changes to the next release though.

It is storage breaking changes, we can't allow releasing things like this without proper storage migration implementation. The breaking changes are acceptable just on the code or API level.

Also, IMHO I'm skeptical about the performance improvements of the store crate; I've checked the code, and there is a scenario where theses collections introduce an extra clone

@karim-en karim-en closed this Feb 24, 2025
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