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.

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

[Persistence] Fix gov parameters schema and enable usability #148

Closed
8 tasks
andrewnguyen22 opened this issue Aug 4, 2022 · 2 comments
Closed
8 tasks
Assignees
Labels
core Core infrastructure - protocol related persistence Persistence specific changes

Comments

@andrewnguyen22
Copy link
Contributor

Objective

Simplfy gov parameters
Adopt the same schema architecture as the actors (sort=desc limit=1 etc.)

Origin Document

The persistence module's parameter schema doesn't work with same block updates.

It is also quite difficult to use to update easily and manage.

Goals / Deliverables

  • Deliver a code complete revamp of gov/params
  • Deliver a document outlining the new architecture of the params and an updated list of them

General issue checklist

  • Update the appropriate CHANGELOG
  • Update the README
  • If applicable, update the source code tree explanation
  • If applicable, add or update a state, sequence or flowchart diagram using mermaid
  • Update any relevant global documentation & references
  • Document small issues / TODOs along the way

Creator: @andrewnguyen22

@Olshansk
Copy link
Member

Olshansk commented Aug 6, 2022

@deblasis is already working on this in #139!

Screen Shot 2022-08-05 at 11 20 25 PM

I'm going to take a first stab at reviewing that PR this weekend but just added you as a reviewer to take another look after

@Olshansk Olshansk added core Core infrastructure - protocol related priority:medium labels Aug 6, 2022
@Olshansk Olshansk moved this to In Progress in V1 Dashboard Aug 6, 2022
@Olshansk Olshansk added this to the V1 Persistence Foundation milestone Aug 6, 2022
@andrewnguyen22
Copy link
Contributor Author

closed with #139

Repository owner moved this from In Progress to Done in V1 Dashboard Aug 21, 2022
andrewnguyen22 pushed a commit that referenced this issue Oct 19, 2022
## Description

A general cleanup issue is needed to tackle TODO's and ensure the persistence module is usable/readable by consolidating `Actor` with `BaseActor` as part of #172. 

Follows issue-#128, issue-#105, issue-#147 and issue-#148 the persistence module is messier and more difficult to understand than the developers would want for organic external contribution.

## Issue

Fixes #210 

## Type of change

Please mark the relevant option(s):

- [ ] New feature, functionality or library
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

- [x] Consolidate `Actor` in `persistence/schema/base_actor.go` with `BaseActor` in `persistence/schema/base_actor.go`.


## Testing

- [x] `make develop_test`
- [x] [LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md) w/ all of the steps outlined in the `README`

## Required Checklist

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have tested my changes using the available tooling
- [x] I have updated the corresponding CHANGELOG

### If Applicable Checklist
- [x] I have updated the corresponding README(s); local and/or global
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s)
- [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core infrastructure - protocol related persistence Persistence specific changes
Projects
Status: Done
Development

No branches or pull requests

2 participants