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

Update to the latest version of celestia-core #175

Merged
merged 10 commits into from
Nov 16, 2021
Merged

Update to the latest version of celestia-core #175

merged 10 commits into from
Nov 16, 2021

Conversation

evan-forbes
Copy link
Member

This PR updates to the latest version of celestia-core. I purposefully limited the changes introduced to be strictly related to upgrading, and did not include any changes related to #130

blocking #130
closes #174

Note, this ivolved swithcing to a version of celestia-core that declares itself as tendermint #528

@evan-forbes evan-forbes self-assigned this Nov 9, 2021
evan-forbes
evan-forbes commented Nov 9, 2021
View reviewed changes
testutils/blockdata.go Outdated Show resolved Hide resolved
Wondertan
Wondertan requested changes Nov 9, 2021
View reviewed changes
go.mod Show resolved Hide resolved
ipld/read.go Show resolved Hide resolved
testutils/blockdata.go Outdated Show resolved Hide resolved
@evan-forbes
Copy link
Member Author

evan-forbes commented Nov 10, 2021 •
edited
Loading

forgot to point out that I moved the testutil code to the proper repo yesterday 0168e02

and then fixed the linter issues as well 9894a1e

adlerjohn
adlerjohn requested changes Nov 10, 2021
View reviewed changes
Copy link
Member

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

Ah nvm there's a conflict with default branch, can you resolve @evan-forbes ?

@evan-forbes
Copy link
Member Author

evan-forbes commented Nov 10, 2021 •
edited
Loading

Ah nvm there's a conflict with default branch, can you resolve

yeah definitely, thanks! ✔️

adlerjohn
adlerjohn previously approved these changes Nov 11, 2021
View reviewed changes
This was referenced Nov 11, 2021
Closed
Merged
liamsi
liamsi previously approved these changes Nov 14, 2021
View reviewed changes
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

THANKS 🚀

@liamsi liamsi dismissed stale reviews from adlerjohn and themself via 070d6a7 November 14, 2021 01:06
@liamsi
Copy link
Member

liamsi commented Nov 14, 2021

I messed up a few things after merging in main. Should be resolved now. But I've copied the test helper generateRandNamespacedRawData to several places.

@liamsi liamsi self-requested a review November 14, 2021 01:30
liamsi
liamsi previously approved these changes Nov 14, 2021
View reviewed changes
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

THANKS 🚀

Wondertan
Wondertan reviewed Nov 14, 2021
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@evan-forbes, We should change the reference form celestial-core to tendermint in Makefile, otherwise LGTM

@evan-forbes
Copy link
Member Author

good catch. I didn't look into exactly how proto generation is handled here, is this correct?
b476f6b

@liamsi
Copy link
Member

liamsi commented Nov 14, 2021

Good catch indeed! I'm not sure if that go list command uses the replace directive or not. If it does, this should be correct.

@Wondertan
Copy link
Member

good catch. I didn't look into exactly how proto generation is handled here, is this correct?

Yeah, thanks! Did you try rerunning the proto command to see if it works?

@evan-forbes
Copy link
Member Author

I did rerun it, but nothing changed. Would someone mind double checking for me to make sure? The generated file is importing using tendermint/tendermint instead of celestaiorg/celestia-core, so I expect it to be working.

@liamsi
Copy link
Member

liamsi commented Nov 15, 2021

I'll check out the branch locally and try it.

@liamsi
Copy link
Member

liamsi commented Nov 15, 2021 •
edited
Loading

Proto generation works as expected.

Wondertan
Wondertan approved these changes Nov 16, 2021
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

If it runs, then it works, no changes should be made

@Wondertan
Copy link
Member

@renaynay, after merging this PR we will need to rebase our PRs and change import manually there in all the new files we write.

renaynay
renaynay approved these changes Nov 16, 2021
View reviewed changes
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Let's merge.

@evan-forbes evan-forbes merged commit e8a4b2c into main Nov 16, 2021
@evan-forbes evan-forbes deleted the evan/update-celestia-core branch November 16, 2021 10:36
renaynay referenced this pull request in renaynay/celestia-node Nov 16, 2021
@evan-forbes @liamsi @renaynay
Update to the latest version of celestia-core (#175)
0c52a84 
* move testutils to this package

* update to latest version of celestia-core

* move over test utils

* update GenFilePV

* move testutil's code to proper repo

* linter

* linter

* fix multiple merge issues

* replace celestia-core with tendermint

Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
renaynay referenced this pull request in renaynay/celestia-node Nov 16, 2021
@evan-forbes @liamsi @renaynay
Update to the latest version of celestia-core (#175)
3e54e19 
* move testutils to this package

* update to latest version of celestia-core

* move over test utils

* update GenFilePV

* move testutil's code to proper repo

* linter

* linter

* fix multiple merge issues

* replace celestia-core with tendermint

Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
renaynay referenced this pull request in renaynay/celestia-node Nov 16, 2021
@evan-forbes @liamsi @renaynay
Update to the latest version of celestia-core (#175)
2c4379e 
* move testutils to this package

* update to latest version of celestia-core

* move over test utils

* update GenFilePV

* move testutil's code to proper repo

* linter

* linter

* fix multiple merge issues

* replace celestia-core with tendermint

Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
@Wondertan Wondertan mentioned this pull request Nov 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liamsi liamsi liamsi approved these changes

@Wondertan Wondertan Wondertan approved these changes

@renaynay renaynay renaynay approved these changes

@adlerjohn adlerjohn adlerjohn left review comments

@mattdf mattdf Awaiting requested review from mattdf

Assignees

@evan-forbes evan-forbes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update to the latest stable version of celestia-core
5 participants
@evan-forbes @liamsi @Wondertan @adlerjohn @renaynay