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

docs: rename message to blob in specs #1617

Merged
merged 11 commits into from
Apr 14, 2023
Merged

docs: rename message to blob in specs #1617

merged 11 commits into from
Apr 14, 2023

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Apr 11, 2023

Closes #1532

@rootulp rootulp self-assigned this Apr 11, 2023
@rootulp rootulp added the specs directly relevant to the specs label Apr 11, 2023
@rootulp rootulp added this to the Mainnet milestone Apr 11, 2023
@github-actions
Copy link

github-actions bot commented Apr 11, 2023 •
edited

PR Preview Action 8825587
🚀 Deployed preview to https://celestiaorg.github.io/celestia-app/pr-preview/pr-1617/
on branch gh-pages at 2023-04-13 19:25 UTC

@rootulp rootulp marked this pull request as ready for review April 11, 2023 20:20
@MSevey MSevey requested a review from a team April 11, 2023 21:15
cmwaters
cmwaters reviewed Apr 12, 2023
View reviewed changes
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

Really like your explanations of the square construction. I think it would be great to add more diagrams supporting the explanations if we can (maybe at a later time). Have added a bunch of minor comments for you to go through.

specs/src/README.md Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Outdated Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Outdated Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Outdated Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Outdated Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Outdated Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Outdated Show resolved Hide resolved
specs/src/rationale/data_square_layout.md Outdated Show resolved Hide resolved
specs/src/specs/block_proposer.md Show resolved Hide resolved
evan-forbes
evan-forbes previously approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

this seems like a pretty strict rename, so I think we're good to merge.

Will defer to other reviewers and the author if they want to increase the scope of this PR slightly when addressing some of the other feedback, which I also think is good (fwiw).

@MSevey MSevey requested a review from a team April 13, 2023 16:21
evan-forbes
evan-forbes previously approved these changes Apr 13, 2023
View reviewed changes
@MSevey MSevey requested a review from a team April 13, 2023 19:25
@rootulp rootulp enabled auto-merge (squash) April 13, 2023 21:17
cmwaters
cmwaters approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments

@rootulp rootulp merged commit 37ca1ef into main Apr 14, 2023
28 checks passed
@rootulp rootulp deleted the rp/specs-message-to-blob branch April 14, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmwaters cmwaters cmwaters approved these changes

@nashqueue nashqueue nashqueue left review comments

@evan-forbes evan-forbes evan-forbes approved these changes

@liamsi liamsi Awaiting requested review from liamsi liamsi is a code owner

@adlerjohn adlerjohn Awaiting requested review from adlerjohn

@MSevey MSevey Awaiting requested review from MSevey

Assignees

@rootulp rootulp

Labels
specs directly relevant to the specs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

specs: rename message => blob
4 participants
@rootulp @cmwaters @evan-forbes @nashqueue