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

Introduce AST error compiler #221

Draft
wants to merge 1 commit into
base: master
from
Draft

Introduce AST error compiler #221

wants to merge 1 commit into from

Conversation

@waiting-for-dev
Copy link
Member

waiting-for-dev commented Jan 8, 2020

From #216 :

It makes message compiler a specific case of a generic error compiler,
just like new ast error compiler.

References dry-rb/dry-validation#604

This is a WIP in regard of adding the possibility to return something different than human readable messages as result errors. It'd be particularly useful in a RESTful scenario when you want your frontend to take care of everything UI-related. I don't want to go any further without having some feedback, as there are some considerations to take into account.

As you can see, I've generalized what it was a MessageCompiler in favor of an ErrorCompiler, being the former a subclass of the latest. I've added an AstErrorCompiler which just returns the plain error ast. Probably it's too raw and we should digest it a little bit, but at the same time it keeps all the flexibility for the machine consuming it.

I've also generalized MessageSet and new AstErrorSet as two subclasses of ErrorSet. As in the ErrorCompiler it makes explicit the interface that dry-schema internals expect for them.

As it'd do the error type polymorphic, probably some result methods should be renamed or aliased, as in the case of #messages.

@solnic

This comment has been minimized.

Copy link
Member

solnic commented Jan 9, 2020

@waiting-for-dev I tweaked rubocop settings, please rebase

It makes message compiler a specific case of a generic error compiler,
just like new ast error compiler.

References dry-rb/dry-validation#604
@waiting-for-dev

This comment has been minimized.

Copy link
Member Author

waiting-for-dev commented Jan 11, 2020

done @solnic

@solnic

This comment has been minimized.

Copy link
Member

solnic commented Jan 11, 2020

@waiting-for-dev seems like this branch is still far behind master...?

@dry-bot

This comment has been minimized.

Copy link
Contributor

dry-bot commented Jan 11, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
- Added 2
           

Coverage decreased per file
===========================
- lib/dry/schema/message_set.rb  -32
         

Complexity increasing per file
==============================
- lib/dry/schema/error_set.rb  3
         

Clones added
============
- spec/unit/dry/schema/message_compiler_spec.rb  1
- spec/unit/dry/schema/error_compiler_spec.rb  1
         

See the complete overview on Codacy

# @api public
setting(:error_compiler, :message)


This comment has been minimized.

Copy link
@dry-bot
@waiting-for-dev

This comment has been minimized.

Copy link
Member Author

waiting-for-dev commented Jan 11, 2020

Damn it!! I started working by error on an old version of master and I fixed it on local in another branch... which is the one I pushed here by mistake. Ok, my bad, now it is the correct branch....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.