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

[9.x] Improving the project structure #1607

Merged
merged 7 commits into from
Apr 14, 2021
Merged

[9.x] Improving the project structure #1607

merged 7 commits into from
Apr 14, 2021

Conversation

andrey-helldar
Copy link
Member

@andrey-helldar andrey-helldar changed the title Improving the project structure [9.x] Improving the project structure Apr 12, 2021
Copy link
Member

@caouecs caouecs left a comment

Choose a reason for hiding this comment

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

A question on Str::contains and PHP8 str_contains function

app/main/Concerns/Contains.php Outdated Show resolved Hide resolved
@caouecs
Copy link
Member

caouecs commented Apr 12, 2021

In TestCase.php
line 64 : return Str::startsWith($filename, 'validation'); => return str_starts_with($filename, 'validation');

lines 29, 34: return Arr::ksort($content); I don't find it in Laravel doc, and it's ksort(), no ?

@andrey-helldar
Copy link
Member Author

andrey-helldar commented Apr 12, 2021

In TestCase.php
line 64 : return Str::startsWith($filename, 'validation'); => return str_starts_with($filename, 'validation');

lines 29, 34: return Arr::ksort($content); I don't find it in Laravel doc, and it's ksort(), no ?

Str::startsWith replaced.

Arr::ksort - this method has never been in the framework. I wrote it myself for my own purposes and use it in several projects. See https://github.com/andrey-helldar/support/blob/main/src/Helpers/Arr.php#L143-L164

Copy link
Member

@caouecs caouecs left a comment

Choose a reason for hiding this comment

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

Just to be sure

app/tests/TestCase.php Outdated Show resolved Hide resolved
@caouecs
Copy link
Member

caouecs commented Apr 13, 2021

It's a major version, I'll merge tomorrow, the time to do all versions

@andrey-helldar
Copy link
Member Author

Laravel-Lang/publisher#84 is ready to work with version 9 and new structure 😊

@caouecs caouecs merged commit c3e51ee into Laravel-Lang:master Apr 14, 2021
@andrey-helldar
Copy link
Member Author

Go go go))

@caouecs
Copy link
Member

caouecs commented Apr 14, 2021

done, but we need to update pull requests :)

@andrey-helldar
Copy link
Member Author

Yes, I wrote in them that the files were changed.

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

Successfully merging this pull request may close these issues.

2 participants