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

Pallet-loans: replace Vec storage by BoundedVec #990

Merged
merged 1 commit into from
Sep 21, 2022
Merged

Pallet-loans: replace Vec storage by BoundedVec #990

merged 1 commit into from
Sep 21, 2022

Conversation

lemunozm
Copy link
Contributor

Description

Replace Vec storage with BoundedVec in pallet-loans to be prepared for Weights V2

Related to: #897

Changes and Descriptions

Modify Vec types found in the pallet-loans storages.

Type of change

  • Bug fix (non-breaking change which fixes an issue). In this case, a future issue.

How Has This Been Tested?

cargo test -p pallet-loans
cargo test --workspace --release --features test-benchmarks,try-runtime

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • I rebased on the latest parachain branch

@lemunozm lemunozm added Q0-trivial An issure which is similar to patching code. P2-nice-to-have Issue is worth doing. D2-notify Pull request can be merged and notification about changes should be documented. labels Sep 21, 2022
@lemunozm lemunozm self-assigned this Sep 21, 2022
@lemunozm lemunozm mentioned this pull request Sep 21, 2022
Closed
4 tasks
gruberb
gruberb suggested changes Sep 21, 2022
View reviewed changes
Copy link
Contributor

@gruberb gruberb left a comment

Choose a reason for hiding this comment

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

Just to clarify why we can remove ensure! the right amount of WriteOffGroups.

pallets/loans/src/functions.rs Show resolved Hide resolved
gruberb
gruberb approved these changes Sep 21, 2022
View reviewed changes
Copy link
Contributor

@gruberb gruberb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lemunozm lemunozm enabled auto-merge (squash) September 21, 2022 07:53
@lemunozm lemunozm merged commit 63cbe8f into parachain Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gruberb gruberb gruberb approved these changes

@hieronx hieronx Awaiting requested review from hieronx

@mustermeiszer mustermeiszer Awaiting requested review from mustermeiszer

@branan branan Awaiting requested review from branan

Assignees

@lemunozm lemunozm

Labels
D2-notify Pull request can be merged and notification about changes should be documented. P2-nice-to-have Issue is worth doing. Q0-trivial An issure which is similar to patching code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lemunozm @gruberb