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: create new validator address (CLI and GUI) #757

Merged
merged 11 commits into from
Oct 24, 2023
Merged

feat: create new validator address (CLI and GUI) #757

merged 11 commits into from
Oct 24, 2023

Conversation

jemiluv8
Copy link
Contributor

@jemiluv8 jemiluv8 commented Oct 20, 2023 •
edited
Loading

Description

Add ability to Create Validator Address in CLI and GUI Applications

Related issue(s)

GUI SCREENSHOT

Screenshot 2023-10-20 at 1 36 32 AM

CLI SCREENSHOT - DEBUGGING AND TESTING

Screenshot 2023-10-20 at 2 08 14 AM

@algora-pbc algora-pbc bot mentioned this pull request Oct 20, 2023
Closed
jemiluv8
jemiluv8 commented Oct 20, 2023
View reviewed changes
Copy link
Contributor Author

@jemiluv8 jemiluv8 left a comment

Choose a reason for hiding this comment

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

@b00f, I just completed a self-review.
This PR is ready for review and any requests for changes.

cmd/gtk/assets/ui/widget_wallet.ui Outdated Show resolved Hide resolved
cmd/gtk/model_wallet.go Outdated Show resolved Hide resolved
cmd/gtk/widget_wallet.go Outdated Show resolved Hide resolved
cmd/gtk/widget_wallet.go Outdated Show resolved Hide resolved
cmd/gtk/widget_wallet.go Outdated Show resolved Hide resolved
cmd/wallet/address.go Outdated Show resolved Hide resolved
wallet/client.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 20, 2023 •
edited
Loading

Codecov Report

Merging #757 (71ee763) into main (34eff4f) will increase coverage by 0.09%.
Report is 6 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #757      +/-   ##
==========================================
+ Coverage   82.72%   82.82%   +0.09%     
==========================================
  Files         173      173              
  Lines        8122     8122              
==========================================
+ Hits         6719     6727       +8     
+ Misses       1078     1072       -6     
+ Partials      325      323       -2

@themantre
Copy link
Contributor

It's better to add a new dialog for creating new address, like:

+----------------------------------------------------------------+
| New address dialog                                             |
+----------------------------------------------------------------+
|                                                                |
| Label: [                               ]                       |
| Type:  [   \ Account               \/  ]                       |
|            \ Validator                                         |
|                                                                |
|                                              [Close] [ OK]     |
+----------------------------------------------------------------+

themantre
themantre requested changes Oct 20, 2023
View reviewed changes
cmd/gtk/model_wallet.go Outdated Show resolved Hide resolved
cmd/gtk/widget_wallet.go Outdated Show resolved Hide resolved
cmd/gtk/assets/ui/widget_wallet.ui Outdated Show resolved Hide resolved
cmd/wallet/address.go Outdated Show resolved Hide resolved
wallet/client.go Outdated Show resolved Hide resolved
cmd/wallet/main.go Outdated Show resolved Hide resolved
cmd/wallet/address.go Outdated Show resolved Hide resolved
@jemiluv8
Copy link
Contributor Author

ok @themantre, I'm working on the changes.

fis: make change requests
5559906 
- introduce modal for new address creation that takes an optional account label and a required account type
- change type flagname to --type for address creation command
- do requested refactoring

TODO: update list of addresses after address modal creates a new address
@themantre
Copy link
Contributor

Good job @jemiluv8 . There are some small linting issues.
Please give me time, I will review the code tomorrow.

@jemiluv8
Copy link
Contributor Author

@themantre I will fix the lint issues. I'm having challenge "refreshing" the
list of addresses after creating a new address from the modal.

Newly created addresses do not show in the addresses table until I switch views
and come back. I'd appreciate any pointers in that regard.
I'm looking into it in the meantime.

@jemiluv8
Copy link
Contributor Author

Fixed issues with newly created address not appearing in the list of addresses.
Also attempted a fix for lint issues. Its ready for review.

@themantre
Copy link
Contributor

@jemiluv8
Please check jemiluv8#1

@pactus-project pactus-project deleted a comment from jemiluv8 Oct 22, 2023
@b00f b00f changed the title feat: Add ability to Create Validator Address in CLI and GUI Applications feat: create new validator address (CLI and GUI) Oct 24, 2023
@b00f b00f merged commit 1804fd3 into pactus-project:main Oct 24, 2023
13 checks passed
@b00f
Copy link
Collaborator

b00f commented Oct 24, 2023

@jemiluv8 Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@themantre themantre themantre requested changes

@b00f b00f b00f approved these changes

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

Successfully merging this pull request may close these issues.

Inability to Create Validator Address in CLI and GUI Applications
3 participants
@jemiluv8 @themantre @b00f