Skip to content

Messages Pallet Documentation Improvement#1981

Merged
wilwade merged 8 commits intomainfrom
docs/messages
May 29, 2024
Merged

Messages Pallet Documentation Improvement#1981
wilwade merged 8 commits intomainfrom
docs/messages

Conversation

@wilwade
Copy link
Collaborator

@wilwade wilwade commented May 22, 2024

Goal

The goal of this PR is to improve the documentation of the Pallets and make that documentation be able to be used on docs.frequency.xyz.

Part of frequency-chain/docs#59

Discussion

  • Messages Pallet Readme

Screenshot

image

@wilwade wilwade requested a review from jeanettedepatie May 22, 2024 19:47
@wilwade wilwade added the documentation Improvements or additions to documentation label May 22, 2024
@codecov
Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
pallets/messages/src/lib.rs 80.53% <100.00%> (ø)

@wilwade wilwade requested review from a team, JoeCap08055, aramikm, claireclark1, enddynayn, mattheworris and shannonwells and removed request for a team May 23, 2024 19:03
@wilwade wilwade marked this pull request as ready for review May 23, 2024 19:04
Co-authored-by: Joe Caputo <joseph.caputo@unfinished.com>
Copy link
Collaborator

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

Nothing blocking, only suggestions

## Summary

Messages stores metadata and message payload data for a set Schema.
Use of the Message pallet payloads is designed for time-centric data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Time-centric? As in, time is the thing a payload revolves around?
Perhaps you mean time-based?

This is also awkward:

Use of the Message pallet payloads...
Perhaps,

Suggested change
Use of the Message pallet payloads is designed for time-centric data.
Message payloads are meant for time-[whatever] data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Trying out this one:

Message payloads are meant for streaming data, where when the message was sent is the focus.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merging for now, but let me know if you have thoughts/better ideas and we can tweak it in one of the other docs PRs.

### Payload Options

- `IPFS`: Storage of the CID and length of the file on IPFS
- `OnChain`: Storage of the entire payload data. Usually for tiny payloads only.
Copy link
Collaborator

Choose a reason for hiding this comment

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

not a big deal but what does tiny mean?

Copy link
Collaborator Author

@wilwade wilwade May 29, 2024

Choose a reason for hiding this comment

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

Updated to:
- OnChain: Storage of the entire payload data, usually for sub-256 byte payloads

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merging for now, but let me know if you have thoughts/better ideas and we can tweak it in one of the other docs PRs.

Copy link
Collaborator

@enddynayn enddynayn left a comment

Choose a reason for hiding this comment

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

great 👍

Copy link
Collaborator

@mattheworris mattheworris left a comment

Choose a reason for hiding this comment

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

Lookin' good, 🚢 it!

@wilwade wilwade enabled auto-merge (squash) May 29, 2024 13:42
@wilwade wilwade merged commit a99aab4 into main May 29, 2024
@wilwade wilwade deleted the docs/messages branch May 29, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants