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

Remove org.json dependency #416

Merged
merged 5 commits into from
Jul 19, 2017

Conversation

kevinhinterlong
Copy link
Member

Addresses issue #340

  • All org.json uses have been replaced with jackson equivalent

@kevinhinterlong
Copy link
Member Author

It looks like a lot of the code around MetricParser is using json where it doesn't need to be. If you think I should change it let me know

Copy link
Contributor

@archolewa archolewa left a comment

Choose a reason for hiding this comment

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

Just one small suggestion

JSONArray expectedJsonObj1
JSONArray expectedJsonObj2
JSONArray expectedJsonObj3
ObjectMapper mapper = new ObjectMapper()
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than building an ObjectMapper directly, I would suggest going through the ObjectMappersSuite. The ObjectMappersSuite That way you're using ObjectMappers with the same configuration as we do throughout the rest of the codebase.

@michael-mclawhorn
Copy link
Contributor

👍

@yahoo yahoo deleted a comment Jul 11, 2017
@yahoo yahoo deleted a comment Jul 11, 2017
@yahoo yahoo deleted a comment Jul 11, 2017
@yahoo yahoo deleted a comment Jul 11, 2017
- All org.json uses have been replaced with jackson equivalent
- Now uses the object mapper configuration used everywhere else in fili-core
- The merge from master should have replaced the previous lines with these
  but somehow it just removed them
@yahoo yahoo deleted a comment Jul 19, 2017
@yahoo yahoo deleted a comment Jul 19, 2017
@yahoo yahoo deleted a comment Jul 19, 2017
@yahoo yahoo deleted a comment Jul 19, 2017
@garyluoex garyluoex merged commit 2e04600 into yahoo:master Jul 19, 2017
kevinhinterlong added a commit to kevinhinterlong/fili that referenced this pull request Jul 19, 2017
@kevinhinterlong kevinhinterlong deleted the removeOrgJsonDependency branch August 8, 2017 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants