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

feat(mailbox): store and return init_peer_id and timestamp [NET-510] #154

Merged
merged 6 commits into from
Jul 27, 2023

Conversation

justprosh
Copy link
Member

@justprosh justprosh commented Jul 21, 2023

Description

Keep init_peer_id for mailbox messages and return timestamps.

Motivation

It's crucial to know who and when pushed messages to the mailbox.

Related Issue(s)

Proposed Changes

  • take init_peer_id from call parameters
  • add MailboxMessage struct:
    data MailboxMessage:
      init_peer_id: string
      timestamp: u64
      message: string
    
  • additionally, move Log and Mailbox data types to dto crate

Checklist

  • The code follows the project's coding conventions and style guidelines.
  • All tests related to the changes have passed successfully.
  • Documentation has been updated to reflect the changes (if applicable).
  • All new and existing unit tests have passed.
  • I have self-reviewed my code and ensured its quality.
  • I have added/updated necessary comments to aid understanding.

Screenshots (if applicable)

[Add any relevant screenshots or animated GIFs to showcase the changes.]

Additional Notes

[Provide any additional information or context that may be helpful for the reviewer.]

Reviewer Checklist

  • Code has been reviewed for quality and adherence to guidelines (https://doc.rust-lang.org/1.0.0/style/README.html).
  • Tests have been reviewed and are sufficient to validate the changes.
  • Documentation has been reviewed and is up to date.
  • Any questions or concerns have been addressed.

@linear
Copy link

linear bot commented Jul 21, 2023

@justprosh justprosh added the e2e Run e2e workflow label Jul 24, 2023
@justprosh justprosh requested review from folex and kmd-fl July 24, 2023 09:38
Copy link
Contributor

@kmd-fl kmd-fl left a comment

Choose a reason for hiding this comment

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

Also, this shouldn't be marked as done

Documentation has been updated to reflect the changes (if applicable).

This change should be documented somewhere, and I don't see that it was written in Spell Service Proposal or near it.

Btw, I don't know how to treat our Spell Service Proposal; it should be just a spell service documentation page, I guess. But it should be changed with spell service changes.

src/spell/modules/spell/spell/src/kv/collection.rs Outdated Show resolved Hide resolved
src/spell/modules/spell/spell/src/mailbox.rs Show resolved Hide resolved
@justprosh justprosh requested a review from kmd-fl July 25, 2023 10:00
@kmd-fl
Copy link
Contributor

kmd-fl commented Jul 25, 2023

The release of this PR will increase the major version of spell service and spell lib, right? Since it's a feat, right?

@justprosh
Copy link
Member Author

The release of this PR will increase the major version of spell service and spell lib, right? Since it's a feat, right?

Yes, because service API has changed

@justprosh justprosh removed the e2e Run e2e workflow label Jul 27, 2023
@nahsi nahsi merged commit 333e404 into main Jul 27, 2023
9 of 10 checks passed
@nahsi nahsi deleted the improve_mailbox branch July 27, 2023 12:17
nahsi pushed a commit that referenced this pull request Jul 27, 2023
🤖 I have created a release *beep* *boop*
---


##
[0.5.17](spell-v0.5.16...spell-v0.5.17)
(2023-07-27)


### Features

* **mailbox:** get_mailbox returns in FIFO order [NET-508]
([#153](#153))
([715981f](715981f))
* **mailbox:** store and return init_peer_id and timestamp [NET-510]
([#154](#154))
([333e404](333e404))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants