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.

Sign up for GitHub

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

Jump to bottom

new cosmwasm doc #1968

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

new cosmwasm doc #1968

wants to merge 11 commits into from

Conversation

costa2400
Copy link

No description provided.

@costa2400 costa2400 changed the title storage key changes new cosmwasm doc Dec 5, 2023
@costa2400 costa2400 requested a review from chipshort December 5, 2023 21:29
webmaster128
webmaster128 reviewed Dec 6, 2023
View reviewed changes
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Thank you!

I think Chris can review the changes when he's back. In the meantime could you run the markdown formatter we use here? ./devtools/format_md.sh This should make the CI happy and makes it easier to review the changes.

@webmaster128
Copy link
Member

Also check out https://editorconfig.org/ and cargo fmt for some of the complains from the CI

@costa2400 costa2400 requested a review from hashedone December 12, 2023 21:49
chipshort
chipshort reviewed Jan 10, 2024
View reviewed changes
Copy link
Collaborator

@chipshort chipshort left a comment

Choose a reason for hiding this comment

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

Nice stuff! This adds doc comments to a lot of types that were missing them.
I left some comments inline to improve.
It would also be good to add the leading space to all of the doc-comments. A lot of the new / changed ones are missing it.

docs/CAPABILITIES.md Outdated Show resolved Hide resolved
docs/STORAGE_KEYS.md Outdated Show resolved Hide resolved
packages/std/src/deps.rs
@@ -15,13 +15,13 @@ pub struct OwnedDeps<S: Storage, A: Api, Q: Querier, C: CustomQuery = Empty> {
pub querier: Q,
pub custom_query_type: PhantomData<C>,
}

/// A mutable version of `Deps` that allows changes to the dependencies.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice to have a doc comment here. I think we can be a bit more specific:

Suggested change
/// A mutable version of `Deps` that allows changes to the dependencies.
/// A mutable version of `Deps` that allows writing to storage.

packages/std/src/errors/std_error.rs Show resolved Hide resolved
packages/std/src/errors/std_error.rs Outdated Show resolved Hide resolved
packages/std/src/results/events.rs Show resolved Hide resolved
packages/std/src/results/submessages.rs Show resolved Hide resolved
packages/std/src/types.rs Show resolved Hide resolved
packages/std/src/errors/std_error.rs Outdated Show resolved Hide resolved
packages/std/src/errors/std_error.rs Show resolved Hide resolved
costa2400 and others added 3 commits January 11, 2024 16:15
@costa2400 costa2400 enabled auto-merge January 11, 2024 21:16
@costa2400 costa2400 closed this Jan 11, 2024
auto-merge was automatically disabled January 11, 2024 21:16

Pull request was closed

@costa2400 costa2400 reopened this Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chipshort chipshort chipshort left review comments

@webmaster128 webmaster128 webmaster128 left review comments

@hashedone hashedone Awaiting requested review from hashedone

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@costa2400 @webmaster128 @chipshort