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

Replace reader tags with functions #14

Merged
merged 4 commits into from
Dec 1, 2023
Merged

Replace reader tags with functions #14

merged 4 commits into from
Dec 1, 2023

Conversation

calherries
Copy link
Contributor

@calherries calherries commented Nov 10, 2023

This PR is a breaking change. It replaces the reader tags defined in data_readers.clj with functions, per Proposal: use malli=? instead of #hawk/malli etc.

Example of how this changes user code:

- (is (=? #hawk/malli [:map [:a :int]] {:a 1})
+ (is (=? (=?/malli [:map [:a :int]]) {:a 1})

See the full set of differences by comparing the docs.

I chose to name the functions to be alphabetic characters only, like malli and not malli= or malli=?, because they are intended to be required into a user code with (require '[mb.hawk.assert-exprs.approximately-equal :as =?]). =?/malli is sufficient to demonstrate its meaning without more characters.

@calherries calherries self-assigned this Nov 10, 2023
@calherries calherries changed the title Replace reader macros with functions Replace reader tags with functions Nov 10, 2023
@calherries calherries requested a review from a team November 10, 2023 16:02
@qnkhuat
Copy link
Contributor

qnkhuat commented Nov 30, 2023

Even though this repo is not really "popular" and afaik we're the only one who uses it, but IMO we should keep the current reader tags to avoid making this a breaking change, there is no downside for us, right? we could mark them as deprecated as well.

@calherries
Copy link
Contributor Author

calherries commented Nov 30, 2023

@qnkhuat It's a fair point, I can't fault that logic. I'll add them back but mark them as discouraged. They still shouldn't need to appear in the documentation.

@camsaul camsaul merged commit 539eefa into main Dec 1, 2023
3 checks passed
@camsaul camsaul deleted the remove-reader-tags branch December 1, 2023 18:57
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.

3 participants