Skip to content

feat: allow instantiation of FormatPHP without config or messages#38

Merged
ramsey merged 1 commit intomainfrom
feature/default-config
Jan 21, 2022
Merged

feat: allow instantiation of FormatPHP without config or messages#38
ramsey merged 1 commit intomainfrom
feature/default-config

Conversation

@ramsey
Copy link
Copy Markdown
Contributor

@ramsey ramsey commented Jan 19, 2022

Description

Allow instantiation of FormatPHP without configuration or message collection instances; FormatPHP will use the system's default locale, in this case.

Product requirements and context

This allows for easier tinkering in composer repl, since it eliminates the need to create locale, config, and message collection objects.

How has this been tested?

PR Checklist

  • I have added tests to cover my changes.

@ramsey ramsey changed the base branch from feature/number-formatter to main January 21, 2022 02:27
@ramsey ramsey force-pushed the feature/default-config branch from bb339ef to e751cf7 Compare January 21, 2022 02:30
@ramsey ramsey force-pushed the feature/default-config branch from e751cf7 to 10cc0e8 Compare January 21, 2022 02:32
@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 10cc0e8 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 97.1% (0.0% change).

View more on Code Climate.

@ramsey ramsey marked this pull request as ready for review January 21, 2022 02:34
@ramsey ramsey requested a review from a team January 21, 2022 02:34
Comment thread tests/FormatPHPTest.php

$formatphpWithoutMessages = new FormatPHP();

$this->assertSame(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This primarily tests that FormatPHP uses the system locale to format the date in the message string, if a locale is not passed in via a Config instance.

@ramsey ramsey requested review from maccath and xiian January 21, 2022 02:37
@ramsey ramsey merged commit 0fad4ce into main Jan 21, 2022
@ramsey ramsey deleted the feature/default-config branch January 21, 2022 15:55
Comment thread src/FormatPHP.php
$this->config = $config;
$this->messages = $messages;
$this->messageFormat = new MessageFormat($config->getLocale());
$this->config = $config ?? new Config(new Locale(PhpLocale::getDefault()));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I wonder if we should set defaults the rest of the way up, so this is just ?? new Config()... and Config has a default of new Locale() and Locale has a default of PhpLocale::getDefault()

Seems like a reasonable set of defaults, but I defer to you.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea. Thanks!

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