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

[5.5] Added "software" as an uncountable word #21324

Merged
merged 1 commit into from
Sep 21, 2017
Merged

Conversation

GrahamCampbell
Copy link
Member

No description provided.

@taylorotwell taylorotwell merged commit 8809580 into 5.5 Sep 21, 2017
@taylorotwell taylorotwell deleted the uncountable-software branch September 21, 2017 20:59
@BrandonShar
Copy link
Contributor

This may be a nit-picky distinction, but should these be considered breaking changes? A few have been added lately, maybe it's time to think about a way to add these on a per-app basis?

@browner12
Copy link
Contributor

does the break occur on things like guessing table names in models?

@BrandonShar
Copy link
Contributor

Yes, it looks to me that it will. Model@getTable calls Str::plural which calls Pluralizer::plural.

@browner12
Copy link
Contributor

i would agree then, yes technically these changes should go to master

@edmandiesamonte
Copy link
Contributor

Assuming we have a model called Software, this is what we do since software's plural form is still software

class Software extends Model {
    
    protected $table = 'software';

With this new change, the code above would still work. Unless otherwise, softwares (with the 's') have already been used (i.e. in migrations), then it is really a breaking change.

Just my two cents.

Disclaimer: I authored a same PR for firmware #21306

@BrandonShar
Copy link
Contributor

Sure, if you specify the table it's going to be fine, but I would guess most people don't do that. If you've been using a softwares table, it will break next time you update.

As a followup, the $uncountable array in Pluralizer is already public, so there's no code change needed to modify it in a service provider on a per app basis.

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.

5 participants