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

[7.x] Rename CastsAttributes contract methods #31075

Closed
wants to merge 2 commits into from
Closed

[7.x] Rename CastsAttributes contract methods #31075

wants to merge 2 commits into from

Conversation

antonkomarev
Copy link
Contributor

@antonkomarev antonkomarev commented Jan 8, 2020

This PR is proposal to rename methods introduced in PR #31035 by @taylorotwell

From my point of view get & set methods are too common and doesn't expose their meaning enough.

This question was raised many times before in different discussion threads. Here is one of the explanations from @sisve #30958 (comment)

I propose to rename them to fromDatabase and toDatabase respectively.

@GrahamCampbell
Copy link
Member

I think get and set are super clear already, because they are totally consistent with the existing terminology. Get is decode and set is encode.

@taylorotwell
Copy link
Member

fromDatabase and toDatabase I found too confusing while prototyping the feature and using it myself. getFooAttribute and setFooAttribute are already how mutators are defined so get and set is consistent with that terminology and is basically impossible to confuse.

@taylorotwell
Copy link
Member

As a follow up, it's very unlikely I'll be considering any "bike shedding" type changes (renaming, etc.) unless there is a VERY good reason to do so. I feel like my PR a few years ago got derailed on those types of things and I'm not going to let that happen again.

@antonkomarev antonkomarev deleted the refactor/rename-casts-attributes-methods branch January 8, 2020 20:56
@andrey-helldar
Copy link
Contributor

andrey-helldar commented Jan 9, 2020

@taylorotwell, as they say in our country - "your own shit does not smell" 😉

PS: this expression is by no means used to insult anything. It is used in cases where someone intentionally denies the existence of a better version of something.

@andrey-helldar
Copy link
Contributor

unless there is a VERY good reason to do so

In other words, there are 100,500 reasons why this will not be done :)

Great strategy! 👏👏👏

(sarcasm)

@laravel laravel locked as too heated and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants