troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4k stars 279 forks source link

ISSUE-1742 Replace kwalify with dry-schema for schema validation #1749

Closed fbuys closed 7 months ago

fbuys commented 7 months ago

This PR looks big but most of the changes is rewriting the old schema file with the dry-schema syntax.

The current validator (Kwalify) seems outdated and lacks good documentation.

A recent issue showed that we could improve the schema validation to also check and warn against missing configurations (See issue: #1734)

dry-schema provides good documentation and it looks like it also provides the features we require.

Structural validation where key presence can be verified separately from values. This removes ambiguity related to "presence" validation where you don't know if value is indeed nil or if a key is missing in the input hash.

Changes include:

See: https://github.com/troessner/reek/issues/1734

We can take a difference approach for https://github.com/troessner/reek/pull/1741 once this is accepted/merged.

This PR should close: https://github.com/troessner/reek/issues/1742

troessner commented 7 months ago

Wow, this looks great! Approved from my side! Wdyt @mvz ?

mvz commented 7 months ago

I'll review this evening.

mvz commented 7 months ago

Just waiting for CI, then it's ready to squash and then merge 🎉

fbuys commented 7 months ago

Just waiting for CI, then it's ready to squash and then merge 🎉

Thank you @mvz the PR is squashed and is ready to merge (if CI passes).

mvz commented 7 months ago

Thanks @fbuys this is a great improvement!