pokt-network / pocket

Official implementation of the Pocket Network Protocol v1
https://pokt.network
MIT License
61 stars 33 forks source link

[Infra] Add app/servicer keys to LocalNet genesis #781

Closed adshmh closed 1 year ago

adshmh commented 1 year ago

Description

Summary generated by Reviewpad on 01 Jun 23 18:05 UTC

This pull request adds 2 applications and 2 servicers to the genesis.json file for LocalNet configuration, which can be used for testing the CLI. It is part of work on issue #754. The patch includes changes to the CHANGELOG.md and README.md files to reflect the added addresses. The relevant options for this pull request are "Documentation" and "Other," specifically "added 2 applications and 2 servicers to genesis.json for LocalNet."

Issue

Part of work on #754

Type of change

Please mark the relevant option(s):

List of changes

Testing

Required Checklist

If Applicable Checklist

adshmh commented 1 year ago

A couple small NITs but otherwise LGTM. Once CI passes, the pattern we use is "Squash & Merge" and copy-paste the PR description into the commit message.

Thank you for the review. Updated the commit message.

adshmh commented 1 year ago

I tried to review the changes since my last review and I'm guessing you git fooed to clean up some of your local commits.

Sorry about that, wasn't local commits, did a push -f by mistake (the CI's timezone does not match mine, so CHANGELOG checks failed on CI after having passed locally).

Olshansk commented 1 year ago

I tried to review the changes since my last review and I'm guessing you git fooed to clean up some of your local commits.

Sorry about that, wasn't local commits, did a push -f by mistake (the CI's timezone does not match mine, so CHANGELOG checks failed on CI after having passed locally).

No worries. Regardless, see my previous comment here about merging it in.

Seems like you might need to merge with master first since there are merge conflicts since this was last reviewed.

Olshansk commented 1 year ago

@adshmh Did you still want to merge this in? If so, it's approved but you'll need to merge with main and resolve conflicts first. I also believe you have the proper github permissions to do so.

Olshansk commented 1 year ago

@adshmh Following up on an offline discussion around merge conflicts related to changelogs.

I took the liberty of resolving the conflict (and pushing the change) and recorded a video as a reference that we can have publically on the steps taken. Let me know if you have any follow up questions on this.

https://github.com/pokt-network/pocket/assets/1892194/c160da23-2854-461f-888f-f162df076dda

Olshansk commented 1 year ago

Per our review guidelines (#773), we always do a "squash and merge" when we merge the PR into main, so the history will be clean on main regardless.

@adshmh Seems like everything is green so feel free to merge if you think it's ready!

adshmh commented 1 year ago

Per our review guidelines (#773), we always do a "squash and merge" when we merge the PR into main, so the history will be clean on main regardless.

@adshmh Seems like everything is green so feel free to merge if you think it's ready!

Done. Thanks!