Skip to content

Spanish (Spain) translation #357

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

Spanish (Spain) translation #357

wants to merge 8 commits into from

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!

datamweb
datamweb requested changes Aug 10, 2022
View reviewed changes
tests/Language/AbstractTranslationTestCase.php Outdated
@@ -49,7 +49,7 @@ abstract class AbstractTranslationTestCase extends TestCase
// BosnianTranslationTest::class => 'bs',
// 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 •
edited
Loading

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

@treborin treborin closed this Aug 11, 2022
@datamweb datamweb mentioned this pull request Aug 11, 2022
Merged
@kenjis
Copy link
Member

kenjis commented Aug 11, 2022

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

@datamweb datamweb mentioned this pull request Aug 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datamweb datamweb Awaiting requested review from datamweb

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@treborin @datamweb @kenjis @MGatner