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

split out JMAP::Client #46

Open
rjbs opened this issue May 29, 2020 · 3 comments
Open

split out JMAP::Client #46

rjbs opened this issue May 29, 2020 · 3 comments

Comments

@rjbs
Copy link
Member

rjbs commented May 29, 2020

Since forever, we have talked about having JMAP::Client as the not-just-for-testing JMAP client. I think this is a good idea. I would like to recommend JMAP::Client over Mail::JMAPTalk.

So, what are the blockers?

  1. I think the Futures branch should be completed and merged first, which has a number of downstream implications. (This should likely be its own project.)

  2. Decide whether JMAP::Tester::Abort is appropriate outside of abortable subtests

  3. Decide whether and how to make JMAP::Typist optional. I believe we should, and the main question is whether it's per-Client or per-request, or both.

  4. Brief pow-wow about how Client would play with future planned expansions like: (a) a DataStore-backed cache engine, a la Overture's (b) registered capabilities and automatic accountId selection (c) smarter snap-in method call id generation to allow methods to make backrefs to previous methods without needing to manage their own call ids globally.

@mmcclimon
Copy link
Contributor

mmcclimon commented May 29, 2020

  1. Yes, agreed (that it should be completed and merged, and maybe that it should be its own project).

  2. I think probably not. (I think even abort in subtests is a bit weird because it doesn't give great output. @wolfsage has complained about this at length, and we should improve it, probably.)

  3. Yes, I think it should be optional, and I think per-client (per-request seems super fiddly).

  4. All of those things sound fun to chat about.

@wolfsage
Copy link
Collaborator

Agreed on all counts, and I do think turning on/off JSON::Typist on a per-client basis makes sense. Per call does seem silly. As far as I can tell we do no validation of the types inside JMAP::Tester ourselves, so if there's for some reason one place where JSON::Typist is not wanted, it's trivial to strip it.

I'm also not against adding it as an option, I just don't see a need (yet).

@rjbs
Copy link
Member Author

rjbs commented Jul 2, 2020

Discussion from a Zoom call, 2020-07-02:

  • exceptions remain abortable

  • exceptions get a parameterized as_string method

  • exceptions get "" overloading

  • exceptions can pretty-print the input that didn't act as expected

  • rjbs prefers Data::Printer, but will not block progress if we use dumpvars.pl

  • we make a use_json_typist attr for the Client, Tester defaults to true

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

No branches or pull requests

3 participants