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

chore: refine sp module #115

Merged
merged 13 commits into from
Mar 23, 2023
Merged

chore: refine sp module #115

merged 13 commits into from
Mar 23, 2023

Conversation

owen-reorg
Copy link
Contributor

@owen-reorg owen-reorg commented Mar 17, 2023 •
edited by jingjunLi
Loading

Description

Refine the sp module according to the quality standard.

Rationale

NA

Example

NA

Changes

Notable changes:

  • add each change in a bullet point here
  • Variable names are clearer， such as modify update_time → update_time_sec, total_deposit's type changed from string to cosmos.base.v1beta1.Coin

@owen-reorg owen-reorg requested a review from fynnss March 17, 2023 04:24
fynnss
fynnss reviewed Mar 17, 2023
View reviewed changes
proto/greenfield/sp/types.proto Outdated Show resolved Hide resolved
x/sp/keeper/genesis.go Outdated Show resolved Hide resolved
x/sp/keeper/params.go Outdated Show resolved Hide resolved
x/sp/keeper/slash.go Outdated Show resolved Hide resolved
x/sp/types/authz.go Outdated Show resolved Hide resolved
x/sp/types/types.go Outdated Show resolved Hide resolved
proto/greenfield/sp/events.proto Outdated Show resolved Hide resolved
fynnss
fynnss reviewed Mar 17, 2023
View reviewed changes
proto/greenfield/sp/events.proto
@@ -19,7 +20,7 @@ message EventCreateStorageProvider {
// endpoint is the domain name address used by SP to provide storage services
string endpoint = 5;
// total_deposit is the token coin that the storage provider deposit to the storage module
string total_deposit = 6;
cosmos.base.v1beta1.Coin total_deposit = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

@krish-nr Please note that this protobuf field changed.

fynnss
fynnss approved these changes Mar 21, 2023
View reviewed changes
Copy link
Contributor

@fynnss fynnss left a comment

Choose a reason for hiding this comment

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

LGTM

x/sp/keeper/msg_server.go Show resolved Hide resolved
@jingjunLi jingjunLi added the r4r label Mar 21, 2023
@owen-reorg
Copy link
Contributor Author

@jingjunLi please refine the PR description.
It seems there are some unsolved comments.

@owen-reorg
Copy link
Contributor Author

LGTM

@fynnss fynnss merged commit 60d3b27 into develop Mar 23, 2023
This was referenced Mar 24, 2023
Merged
Merged
@unclezoro unclezoro deleted the quality/sp branch April 18, 2023 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jingjunLi jingjunLi jingjunLi left review comments

@fynnss fynnss fynnss approved these changes

@RenRick RenRick Awaiting requested review from RenRick

@unclezoro unclezoro Awaiting requested review from unclezoro

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

Successfully merging this pull request may close these issues.
3 participants
@owen-reorg @jingjunLi @fynnss