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

Library review and improvements #18

Merged
merged 4 commits into from
May 26, 2023
Merged

Library review and improvements #18

merged 4 commits into from
May 26, 2023

Conversation

visciang
Copy link
Contributor

Hello @corka149,
thank you for the library!

We have evaluated the library while working on some JSON API related features.
During the evaluation we have fixed some bugs, extended some functionalities, etc.

The result is a major (2.0) jump, since some API signature have been changed, check out the CHANGELOG
for a summary.

We are currently using our internal fork of the library but we would like to contribute back.
Let us know if this can be accepted or not.

Thank you,
the Athonet Team ❤️ ❤️ ❤️

Giovanni Visciano and others added 2 commits May 22, 2023 14:48
@corka149
Copy link
Owner

Hey @visciang . Thanks for the feedback and huge effort!

I am currently short on time and will try to have a look at it on the weekend.

@corka149 corka149 self-requested a review May 23, 2023 19:03
Copy link
Owner

@corka149 corka149 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of changes 😅 . I iterated over it the first time.

lib/jsonpatch/utils.ex Show resolved Hide resolved
lib/jsonpatch/utils.ex Show resolved Hide resolved
lib/jsonpatch_exception.ex Outdated Show resolved Hide resolved
CHANGELOG Outdated Show resolved Hide resolved
mix.exs Outdated Show resolved Hide resolved
Giovanni Visciano and others added 2 commits May 23, 2023 23:34
Copy link
Owner

@corka149 corka149 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the changes.

The only nitpicking I have is about naming a module only Util. The name is always an invitation for placing unrelated functions in one module. But I am fine for now with this as the scope of the lib is very small.

@corka149 corka149 merged commit f0a41e2 into corka149:master May 26, 2023
@visciang
Copy link
Contributor Author

visciang commented May 26, 2023

@corka149 may I ask you to push 2.0.0 on hex.pm?

Thank you
Giovanni

@corka149
Copy link
Owner

@visciang already done ;-)

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