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

28-integration-in-ci #29

Closed
wants to merge 2 commits into from
Closed

28-integration-in-ci #29

wants to merge 2 commits into from

Conversation

khalyomede
Copy link

This PR adds a new --ci (or -c) flag to instruct the command to return a non zero code in case some untranslated keys have been found.

self::flushDirectories('lang', 'views');
}

public function testLocalizeCommandReturnsNonZeroCodeThenZeroCodeIfFileDidNotChangedBetweenTwoRun(): void
Copy link
Author

Choose a reason for hiding this comment

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

This one will not pass, because I feel I will need something new that seems to not exist yet. Let me explain:

For this test to pass, the parser should detect, among the keys it has collected, which ones are not yet in all translated files. This seems like this piece of logic mixes both the Parser and the Writers, since the Writers are aware of which translations files contains which keys, when the Parser only knows which keys are in the "app" files.

My first suggestion would be to have a method Parser::foundSomeNewKeysComparedTo(Collection $keys): bool.

What do you think about it @amiranagram?

Copy link
Owner

Choose a reason for hiding this comment

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

Good job overall!

As for this you can use Amirami\Localizator\Services\Localizator class to merge the keys together and figure out if there are any. Look at the collect method there, it does everything. You can make it public.

So like:

$localizator->collect($parser->getKeys($locale, $type), $type, $locale);

Obviously need to loop over all types and locales.

@amiranagram amiranagram linked an issue Jan 24, 2022 that may be closed by this pull request
@amiranagram amiranagram marked this pull request as draft February 2, 2022 08:24
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.

Integration in CI
2 participants