storage-plus: containers

[uint]

Review and merge #20 first!

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
cosmwasm-docs	✅ Ready (Inspect)	Visit Preview	💬 6 unresolved ✅ 6 resolved	Jun 17, 2024 1:58pm

[aumetra]

By the way, you should add the crates you added as dependencies to the GitHub CI workflow, too, to update them to the latest versions (otherwise they are locked by the lockfile)

[dakom]

docs look good to me fwiw :) left a couple small questions, nothing major

[hashedone]

Overall I would make sure the documentation is not simply list of functions and then some random examples - it has to be easy to follow by someone never using this thing before. Going to every single useful function one by one and providing an example would be easier to understand in my opinion.
Also we cannot just focus on simple concepts, and then ignore things like prefixed and layered things assuming that people would figure it out - we need to take special care about explaining those non-trivial things.

[hashedone]

Why this way instead of importing stuff directly in the file?

[uint]

So that we don't have to add #[allow(unused_imports)] to every use directive if we decide to add more. It looks awful.

[aumetra]

#![allow(unused_imports)] at the top of the file once

[uint]

@aumetra My instinct is to be more granular so that we still get warnings in other places, where they might be useful feedback.

[uint]

@hashedone I agree mostly. My goal with this PR was to braindump content, honestly, and review it and rearrange it in subsequent PRs.

I'm currently fighting my perfectionism, so I'm trying not to perfect every aspect of everything before I deliver it ;) It's usually a mistake.

[uint]

Rebased

[uint]

I addressed the comments from @dakom.

@hashedone Your review includes items that are both high-level and low-level, making it a little difficult to address. I think overall it's suggesting a rewrite, which I also think is due.

I don't currently have the throughput to do that - I'm in the middle of writing the IndexedMap thing + other stuff, and won't context-switch to doing a full rewrite here. I've taken note of all the feedback.

I strongly suggest we merge this as it is (even if not perfect), and iterate on this in subsequent PRs to keep up a sense of progress and achievement here, rather than getting stuck. If that's not OK with you, just close this and I'll submit a new PR in the future.

[uint]

Also we cannot just focus on simple concepts, and then ignore things like prefixed and layered things assuming that people would figure it out - we need to take special care about explaining those non-trivial things.

Ah, a question about that: in my head, the priority list goes something like:

1. Explain cw-storage-plus basic-to-medium topics
2. Explain storey fairly in-depth
3. Explain cw-storage-plus advanced topics

Is that wrong? I worry if we spend all the time explaining cw-storage-plus advanced topics now (there's a fair bit), we risk running out of time to touch storey, and that does not seem exactly aligned with our vision.

Which bits can we not have assuming resources run out? Trying to think strategically here.

[hashedone]

I addressed the comments from @dakom.

@hashedone Your review includes items that are both high-level and low-level, making it a little difficult to address. I think overall it's suggesting a rewrite, which I also think is due.

I don't currently have the throughput to do that - I'm in the middle of writing the IndexedMap thing + other stuff, and won't context-switch to doing a full rewrite here. I've taken note of all the feedback.

I strongly suggest we merge this as it is (even if not perfect), and iterate on this in subsequent PRs to keep up a sense of progress and achievement here, rather than getting stuck. If that's not OK with you, just close this and I'll submit a new PR in the future.

Lets go this way. Go through my comments. For low-level, things, whatever is easy to address - please just apply and resolve a comment. For things that are more high-level, annoying to fix in the particular review - simply open a PR, and we will figure it out there.

[uint]

Rebased

[hashedone]

I was thinking how to approach this, mostly because of this comment and your response, but for now I think the best would be to merge it and move forward.