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

Refactoring-code #539

Merged
merged 1 commit into from
Mar 27, 2022
Merged

Refactoring-code #539

merged 1 commit into from
Mar 27, 2022

Conversation

Sahil3198
Copy link
Contributor

Here I made some changes to the code to improve code readability.
I used some refactoring Techniques such as Extract Method, Rename variable, Pull-up variable, Extract class, Replace conditional with polymorphism, and Push-down method.

@stevehu stevehu merged commit ca36ea4 into networknt:master Mar 27, 2022
@stevehu
Copy link
Contributor

stevehu commented Mar 27, 2022

@Sahil3198 Thanks a lot for your help. I didn't realize that we have the validationContext in almost all the validators as it was introduced for only one or two validators in the beginning and almost every validator is using it these days. It makes perfect sense to move to the BaseJsonValidator. Also, extracting the different versions of the implementation paves the road for use to support new versions easily.

@TJC
Copy link

TJC commented Apr 4, 2022

Release 1.0.68 introduced a nasty memory leak. There were only a couple of significant PRs included in that release, and this was one of them. See #546 for details. I'm not really sure how this PR could have triggered it, but thought I'd mention the problem.

@TJC
Copy link

TJC commented Apr 4, 2022

Ah, actually my git bisect points to this commit 5007242 as the cause.

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