Skip to content

[Translation] added option json_options to JsonFileDumper. #15756

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

Merged
merged 1 commit into from
Sep 11, 2015

Conversation

aitboudad
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets ~
License MIT
Doc PR

Replaces #14750

@@ -23,9 +23,23 @@ class JsonFileDumper extends FileDumper
/**
* {@inheritdoc}
*/
protected function formatCatalogue(MessageCatalogue $messages, $domain, array $options = array())
{
if (isset($options['json_encoding'])) {
Copy link
Member

Choose a reason for hiding this comment

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

maybe call this json_flags or json_options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

json_flags sound good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but can we take on #14750 (comment) ?

Copy link
Member

Choose a reason for hiding this comment

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

OK, then fine for json_encoding...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted, thanks!

@nicolas-grekas
Copy link
Member

👍
Status: reviewed

@@ -30,7 +30,19 @@ public function testDump()
$dumper->dump($catalogue, array('path' => $tempDir));

$this->assertEquals(file_get_contents(__DIR__.'/../fixtures/resources.json'), file_get_contents($tempDir.'/messages.en.json'));
unlink($tempDir.'/messages.en.json');
Copy link
Member

Choose a reason for hiding this comment

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

this won't be unlinked in case of assertion failure. You should do it on teardown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

It's passed to json_encode function second argument.
@aitboudad
Copy link
Contributor Author

Thank you @gepo.

@aitboudad aitboudad merged commit bfdc354 into symfony:2.8 Sep 11, 2015
aitboudad added a commit that referenced this pull request Sep 11, 2015
…per. (gepo)

This PR was merged into the 2.8 branch.

Discussion
----------

[Translation] added option json_options to JsonFileDumper.

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | ~
| License       | MIT
| Doc PR        |

Replaces #14750

Commits
-------

bfdc354 [Translation] added option flags to JsonFileDumper. It's passed to json_encode function second argument.
@aitboudad aitboudad deleted the pr-14750 branch September 11, 2015 11:35
@fabpot fabpot mentioned this pull request Nov 16, 2015
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.

6 participants