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

fluent/quip: remove in favor of qp #197

Merged
merged 1 commit into from
Jul 1, 2021
Merged

fluent/quip: remove in favor of qp #197

merged 1 commit into from
Jul 1, 2021

Conversation

mvdan
Copy link
Contributor

@mvdan mvdan commented Jun 18, 2021

(see commit message)

After experimenting with quip and qp for a few months, we seem to agree
that qp is a bit nicer to use. Remove quip, since it's largely redundant
going forward.

Since the qp docs referenced quip, redo that to stand on its own ground.
@mvdan mvdan requested a review from warpfork June 18, 2021 09:09
@warpfork
Copy link
Collaborator

Yep. I give up on this package.

Some more comments on lessons learned, for any future review on this, or for future attempts on these fluent APIs:

  • The strategy of error handling quip attempted was intended to score high on explicitness. It's an interesting idea. But:
    • It failed to see that explicitness translate into reliably better error handling or better DX.
    • The odds of making mistakes in the error gathering, which is left largely up to you as the user, is just way way too great.
      • My own attempts to use it have run into this so frequently that there's just... no. No way.
  • The outcomes when you do have an error gathering mistake are just too terrible: you'll get "misuse" panics from the Node implementation's guts, but at an arbitrary distance from the problem code. The result is utterly horrible to debug.
  • The composition strategy when starting to use your own functions for repeated pieces of data wrangling is just too odd.
    • The least-weird way to go about this probably involved repeating the pattern of an error pointer parameter in your own functions. Which is very unusual in golang, to put it mildly. I didn't expect this to be contagious. That it turned out to be is problematic.

@warpfork
Copy link
Collaborator

Some other tangentially related lessons learned during my day trying to work with this package (not strictly related, but I want to note before I forget): it would be nice to have a clone method on assemblers, or other way to inspect partially built state, so you can stop and take stock of the situation. This is currently kind of hard to do, because that API was driven by efficiency concerns and by trying to shield incomplete/not-yet-validated data from being read, but lacking this during debugging is painful.

@mvdan mvdan merged commit b7347f1 into ipld:master Jul 1, 2021
@mvdan mvdan deleted the remove-quip branch July 28, 2021 15:45
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