Skip to content

Conversation

@treborin
Copy link
Contributor

No description provided.

@datamweb
Copy link
Collaborator

Hey @treborin ,
Thank you for contributing!
Please see #235 (comment)

@kenjis kenjis added the enhancement New feature or request label Aug 10, 2022
@treborin
Copy link
Contributor Author

Hey @treborin , Thank you for contributing! Please see #235 (comment)

@datamweb . Done!

// CzechTranslationTest::class => 'cs',
GermanTranslationTest::class => 'de',
// SpanishTranslationTest::class => 'es',
SpanishTranslationTest::class => 'es',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please fix the coding style. just run :
composer cs-fix

@datamweb
Copy link
Collaborator

It looks like you are using GitHub UI to post this PR.
Please rename file tests/Language/SpanishTranslationTest to tests/Language/SpanishTranslationTest.php.
Then tell me if there is any problem in executing command composer cs-fix?

@treborin
Copy link
Contributor Author

It looks like you are using GitHub UI to post this PR. Please rename file tests/Language/SpanishTranslationTest to tests/Language/SpanishTranslationTest.php. Then tell me if there is any problem in executing command composer cs-fix?

image

@datamweb
Copy link
Collaborator

datamweb commented Aug 10, 2022

image

@kenjis , Does executing command composer update solve this problem?

@treborin Problem coding style is fixed.
Now we only have one error in the unit test.
Let kenjis continue to be with you.
And a curious question, did you run composer cs-fix in the main directory of the shield?

@treborin treborin requested a review from datamweb August 10, 2022 11:14
@kenjis
Copy link
Member

kenjis commented Aug 10, 2022

@treborin run composer update and composer cs-fix in the repository root directory, that is shield.

@treborin
Copy link
Contributor Author

@treborin run composer update and composer cs-fix in the repository root directory, that is shield.

@kenjis , cmposer update, ok. But here is composer cs-fix:
image

@datamweb, i fixed coding style by hand. I saw inside unsuccessfuls checks thay te problem was the file text format, that need to be ansii. And i fixed it by hand, line by line.

@datamweb
Copy link
Collaborator

Yes, I got it.
try
mkdir build
composer update
composer cs-fix

@treborin
Copy link
Contributor Author

Yes, I got it. try mkdir build composer update composer cs-fix

image
image

@MGatner
Copy link
Member

MGatner commented Aug 11, 2022

The cs-fix command was just added this week (#340). If your fork was from earlier the command would be composer style. Either way that is simply an alias, you can run the command directly:

php vendor/bin/php-cs-fixer fix --ansi

@MGatner
Copy link
Member

MGatner commented Aug 11, 2022

I don't know what is happening with those action failures. The tests all pass but a PHP error occurs when it tries to gather coverage. I believe it is unrelated to this PR.

@treborin
Copy link
Contributor Author

I don't know what is happening with those action failures. The tests all pass but a PHP error occurs when it tries to gather coverage. I believe it is unrelated to this PR.

I try with a new PR

@kenjis
Copy link
Member

kenjis commented Aug 11, 2022

@MGatner The PHPUnit error is related to this PR.
See #364 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants