Skip to content

Conversation

@Cazen
Copy link
Contributor

@Cazen Cazen commented Dec 28, 2015

We can provides the option to choose JSON parser can be enabled to accept quoting of all character or not.

@Cazen
Copy link
Contributor Author

Cazen commented Dec 28, 2015

recreate pull request(10496->10497)

@srowen
Copy link
Member

srowen commented Dec 28, 2015

In general you do not need to open a new pull request. But before you make a PR, best to finish the questions on the JIRA. I'm not yet clear why this is regularly needed, or when

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think you need the trailing \ yet

@Cazen
Copy link
Contributor Author

Cazen commented Dec 30, 2015

Changed default value to true and remove \ in comment

@SparkQA
Copy link

SparkQA commented Dec 30, 2015

Test build #2264 has finished for PR 10497 at commit abdbb58.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

Choose a reason for hiding this comment

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

This default should be false, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy New Year Owen!

At first, I've created PR with false default.

But Xin advised to me that "I'd actually change the default value to true" in previous comment so I've changed.

If you think it doesn't make sense, please call me again. I will change that immediately

Thank you

@Cazen
Copy link
Contributor Author

Cazen commented Jan 3, 2016

Could I ask to run test?

@SparkQA
Copy link

SparkQA commented Jan 4, 2016

Test build #2308 has finished for PR 10497 at commit e75ff1d.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Jan 4, 2016

I've merged this. Thanks!

@asfgit asfgit closed this in b8410ff Jan 4, 2016
@rxin
Copy link
Contributor

rxin commented Jan 4, 2016

@Cazen you probably want to add this email to your github profile so the commit shows up under your account: [email protected]

@Cazen
Copy link
Contributor Author

Cazen commented Jan 4, 2016

Thank you @rxin I've added my email address to github profile.
I'm very pleased with your help. Have a good day!

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.

4 participants