-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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
Conversation
@@ -23,9 +23,23 @@ class JsonFileDumper extends FileDumper | |||
/** | |||
* {@inheritdoc} | |||
*/ | |||
protected function formatCatalogue(MessageCatalogue $messages, $domain, array $options = array()) | |||
{ | |||
if (isset($options['json_encoding'])) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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) ?
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted, thanks!
👍 |
@@ -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'); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Thank you @gepo. |
…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.
Replaces #14750