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

refactor: add note to deliver note #10482

Open
wants to merge 1 commit into
base: ek/refactor/pxe-cleanup/remove-get-outgoing-notes-from-interface
Choose a base branch
from

Conversation

sklppy88
Copy link
Contributor

@sklppy88 sklppy88 commented Dec 7, 2024

In this PR, we are renaming pxe.addNote to pxe.deliverNote. The rationale was because in this function, we aren't actually 'adding a note' to state, as this can only be used when a note (hash) already exists. This is used to deliver the existing note (most likely to self), and (most likely) when said note was not emitted.

Copy link
Contributor Author

sklppy88 commented Dec 7, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sklppy88 sklppy88 changed the title init refactor: add note to deliver note Dec 7, 2024
@sklppy88 sklppy88 force-pushed the ek/refactor/pxe-cleanup/remove-get-outgoing-notes-from-interface branch from 02e0cf0 to e7dbfc8 Compare December 8, 2024 19:59
@sklppy88 sklppy88 force-pushed the ek/refactor/pxe-cleanup/rename-add-note-to-deliver-note branch from 5168e57 to 390329a Compare December 8, 2024 19:59
@sklppy88 sklppy88 force-pushed the ek/refactor/pxe-cleanup/remove-get-outgoing-notes-from-interface branch from e7dbfc8 to a0cd02c Compare December 8, 2024 20:18
@sklppy88 sklppy88 force-pushed the ek/refactor/pxe-cleanup/rename-add-note-to-deliver-note branch from 390329a to 38e8ae9 Compare December 8, 2024 20:18
@sklppy88 sklppy88 force-pushed the ek/refactor/pxe-cleanup/remove-get-outgoing-notes-from-interface branch from a0cd02c to 98973ff Compare December 8, 2024 20:34
@sklppy88 sklppy88 force-pushed the ek/refactor/pxe-cleanup/rename-add-note-to-deliver-note branch from 38e8ae9 to 9277991 Compare December 8, 2024 20:34
@sklppy88 sklppy88 force-pushed the ek/refactor/pxe-cleanup/rename-add-note-to-deliver-note branch from 9277991 to 9cfd934 Compare December 8, 2024 20:40
@sklppy88 sklppy88 marked this pull request as ready for review December 8, 2024 20:44
@iAmMichaelConnor
Copy link
Contributor

I actually prefer addNote. "Deliver" suggests you are sending the note to someone else; but you're not; you're just adding the note to your pxe.

@sklppy88
Copy link
Contributor Author

sklppy88 commented Dec 8, 2024

I actually prefer addNote. "Deliver" suggests you are sending the note to someone else; but you're not; you're just adding the note to your pxe.

I think during the discussion we preferred "deliver" over "add" because the note is not necessarily added, as the note hash must already exist for this to work. So in essence you may use this to "deliver" an already "added" / "created" note (that may not have been emitted) to yourself. Maybe we can think of a better name ?

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.

2 participants