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

feat: update x/stakingplus to use Finschia/cosmos-sdk #1225

Merged
merged 31 commits into from
Feb 16, 2024
Merged

feat: update x/stakingplus to use Finschia/cosmos-sdk #1225

merged 31 commits into from
Feb 16, 2024

Conversation

ulbqb
Copy link
Member

@ulbqb ulbqb commented Feb 2, 2024 •
edited
Loading

Description

closes: #1200

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml
  • I have updated the documentation accordingly.

@ulbqb ulbqb self-assigned this Feb 2, 2024
@ulbqb ulbqb marked this pull request as ready for review February 2, 2024 08:31
@ulbqb ulbqb requested a review from a team as a code owner February 2, 2024 08:31
tkxkd0159
tkxkd0159 requested changes Feb 2, 2024
View reviewed changes
x/stakingplus/README.md Outdated Show resolved Hide resolved
x/stakingplus/keeper/keeper_test.go Outdated Show resolved Hide resolved
x/stakingplus/keeper/keeper_test.go Outdated Show resolved Hide resolved
@tkxkd0159
Copy link
Member

I have two more questions.

  1. Is there any reason to not implement e2e test for the special case of stakingplus (createValidator)?
  2. Is there any reason to not apply this at app_v2?

@ulbqb @tkxkd0159
Update x/stakingplus/README.md
dcdcfa2 
Co-authored-by: Jayden Lee <41176085+tkxkd0159@users.noreply.github.com>
@ulbqb
Copy link
Member Author

ulbqb commented Feb 5, 2024 •
edited
Loading

@tkxkd0159

Is there any reason to not implement e2e test for the special case of stakingplus (createValidator)?

This time I simply converted the existing stakingplus module. But it seems necessary so I'll add the test.

Is there any reason to not apply this at app_v2?

https://github.com/ulbqb/finschia-sdk/blob/dcdcfa204220529bc1f308f56523b967edfcacc1/simapp/app_config.go#L78
The initialization of the stakingplus keeper is done here. The keeper interface is not changed so there are few changes.

0Tech
0Tech requested changes Feb 6, 2024
View reviewed changes
x/stakingplus/keeper/keeper_test.go Outdated Show resolved Hide resolved
x/stakingplus/authz.go Outdated Show resolved Hide resolved
@ulbqb ulbqb requested a review from 0Tech February 13, 2024 05:16
0Tech
0Tech previously approved these changes Feb 13, 2024
View reviewed changes
Copy link
Collaborator

@0Tech 0Tech 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/stakingplus/README.md Outdated Show resolved Hide resolved
@ulbqb @0Tech
Update x/stakingplus/README.md
a704727 
Co-authored-by: Youngtaek Yoon <noreply@yoon.mailer.me>
@ulbqb ulbqb requested a review from 0Tech February 13, 2024 10:02
0Tech
0Tech previously approved these changes Feb 13, 2024
View reviewed changes
jaeseung-bae
jaeseung-bae previously approved these changes Feb 14, 2024
View reviewed changes
Copy link
Contributor

@jaeseung-bae jaeseung-bae left a comment

Choose a reason for hiding this comment

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

LGTM

@ulbqb ulbqb dismissed stale reviews from jaeseung-bae and 0Tech via 72ad428 February 16, 2024 03:12
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Feb 16, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@tkxkd0159 tkxkd0159 self-requested a review February 16, 2024 03:19
@ulbqb ulbqb merged commit 2cc0f1b into Finschia:bumpup50 Feb 16, 2024
16 of 17 checks passed
@ulbqb ulbqb deleted the bumpup50-stakingplus branch February 16, 2024 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaeseung-bae jaeseung-bae jaeseung-bae approved these changes

@tkxkd0159 tkxkd0159 tkxkd0159 approved these changes

@0Tech 0Tech 0Tech approved these changes

Assignees

@ulbqb ulbqb

Labels
A: build A: CI C:x/stakingplus
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ulbqb @tkxkd0159 @0Tech @jaeseung-bae