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
Serialize warnings #45
Conversation
|
||
;; send the client response, whether success or failure | ||
(%push-raw-request receiver identity empty-frame (serialize reply))))) | ||
(handler-case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Emacs reindented this for me, but the real change is the handler-bind
and the :|warnings|
slot initializers.
9b4986e
to
ffcb9b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small things; looks fine overall.
src/rpcq.lisp
Outdated
(defmethod %serialize ((payload cons)) | ||
(loop :for elt :in payload :collect (%serialize elt))) | ||
(map 'vector #'%serialize payload)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we nix serializing conses altogether?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you were to ask me: yes. If you were to ask someone else with more principles about backwards compatibility: no, or not yet.
(defmethod %deserialize ((payload cons)) | ||
(loop :for elt :in payload :collect (%deserialize elt))) | ||
(map 'vector #'%deserialize payload)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question here
3e3d16a
to
184e414
Compare
This LGTM. What changes ripple through the stack? |
AFAICT, this is the only ripple: quil-lang/quilc#188 . |
let's merge after the weekend |
Weekend is done; anybody want to merge? (I'm not a CODEOWNER.) |
Only serious concern is whether this will swallow warnings in the server logs. |
ty everyone |
Closes #43: captures Server-side warnings and re-raises them on the Client side.