Skip to content

Supports ever changing lang folder#170

Merged
rmariuzzo merged 2 commits intormariuzzo:masterfrom
wilpat:master
Aug 26, 2022
Merged

Supports ever changing lang folder#170
rmariuzzo merged 2 commits intormariuzzo:masterfrom
wilpat:master

Conversation

@wilpat
Copy link
Copy Markdown
Contributor

@wilpat wilpat commented Aug 25, 2022

From laravel 4 to 8.x we've seen changes to where the lang folder lives and this breaks the functionality of this package.

This PR makes this path customisable, so the package does not need to be updated every time laravel makes an update to the location of this folder.

Copy link
Copy Markdown
Owner

@rmariuzzo rmariuzzo left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for your collaboration.

@rmariuzzo rmariuzzo merged commit 9e570ee into rmariuzzo:master Aug 26, 2022
@rmariuzzo
Copy link
Copy Markdown
Owner

@Chomiciak
Copy link
Copy Markdown

Guys please... Not the Fix Friday again! @wilpat @rmariuzzo

image

My advice is to enable at least double review, if anyone (e.g. our projects) has the dependency set to ^1.9 you just broke all the CI/CD. Also don't do updates on Fridays so people don't have to work weekends to resolve possible issues 😭

Please flag the version as non-working as soon as possible.

@rmariuzzo
Copy link
Copy Markdown
Owner

rmariuzzo commented Aug 26, 2022

@Chomiciak I apologize for this, that's on me. I removed the broken version from Packagist, I think that should alleviate that issue. Please, let me know if not.

@rmariuzzo
Copy link
Copy Markdown
Owner

Also, I appreciate your advice, I will make a call for more maintainers to make double review as you suggested.

rmariuzzo added a commit that referenced this pull request Aug 26, 2022
$langs = $app['path.base'].'/app/lang';
} elseif ($laravelMajorVersion >= 5) {
$languagePath = 'resources/lang';
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Its better to use app()->langPath() instead of all this.

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.

4 participants