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

Change "Facade" to "Surrogate" #3835

Closed
wants to merge 14 commits into from
Closed

Change "Facade" to "Surrogate" #3835

wants to merge 14 commits into from

Conversation

pmjones
Copy link

@pmjones pmjones commented Mar 8, 2014

This backwards-compatible change removes the word "Facade" in class names and comments in favor of the word "Surrogate." All current tests pass.

The word "Facade" implies the facade pattern, but the implementation is closer to a proxy pattern. The word "Surrogate", on the other hand, implies something closer to the existing behavior (i.e., a static proxy call to a container instance).

The method names (get|set)Facade(Accessor|Application) remain in place. Changing them requires changing the related abstract class (Surrogate, née Facade). Because userland code may be extending the abstract class, that would require the userland code to be changed as well. That is not a backwards-compatible approach. Thus, the method names remain as a historical artifact. Future versions should deprecate and/or rename these methods to remove the word Facade.

Per https://github.com/laravel/framework#contributing: Since this is a rename/replace of an existing behavior, and not a new component, this pull request is against the latest stable release. Please provide specific instructions if another approach is preferable.

@taylorotwell
Copy link
Member

I told you we would make a documentation change. Not a code change.

@joshmanders
Copy link
Contributor

This is not backwards compatible, it would break all packages that extend the facade class.

@taylorotwell
Copy link
Member

To elaborate on the craziness that will certainly follow this request. I have specifically said we are not making a code change to rename this right now, and that I would consider it on a following major release, but probably still won't make the change. We certainly won't make the change based on demands from outside our community.

Secondly, this is not backwards compatible, as it totally deletes a class some users may already be extending.

Edit: correction. It is backwards compatible. However, still see no need to make this change, as I've stated before. Certainly not from demands from those outside of the Laravel community.

@taylorotwell
Copy link
Member

No further pull request attempts to rename facades should be made. They will be rejected.

@robclancy
Copy link
Contributor

Oh god. These idiots still going on about this? No wonder other programming communities look down on PHP. It's a word. Which fits the definition of what it is doing.

@pmjones
Copy link
Author

pmjones commented Mar 8, 2014

It is in fact backwards-compatible; I have added a new test here to prove that assertion:

https://github.com/pmjones/framework/blob/4.1/tests/Support/SupportSurrogateTest.php#L20-L24

I would update the range if I could; I'll make another pull request for you to refuse on the record, if you like.

@taftse
Copy link

taftse commented Mar 8, 2014

@pmjones why not just focus on your own framework and leave us laravel developers in peace

@taylorotwell
Copy link
Member

I have stated on Twitter and on Reddit multiple times we are not making this change.

@robclancy
Copy link
Contributor

This image sums up everyone who wants this word changed.
fishing

@cordoval
Copy link

cordoval commented Mar 8, 2014

Here to support @pmjones and freedom of speech 👶

@robclancy
Copy link
Contributor

How ironic.

@cordoval
Copy link

cordoval commented Mar 8, 2014

I know you meant it because you can name or use the word you want and misinform. But like a good sect is not based on reason and forbids incoming exhortations. I meant it because GitHub is a free world and so it is FOSS.

@robclancy
Copy link
Contributor

Stop making words, you're bad at it.

@AmyStephen
Copy link

@pmjones That PR has "facade" throughout, anyway, so it's not even a consistent change. This is a term issue that the community needs to consider. Is it worth migrating to a new term? If so, how and when should the change be introduced? What coordination issues are involved? Should it come at a full version change, etc.? The code, itself, is likely the least complicated part. They have a conference in NYC in May where I'm certain the issue will come up and they can talk about all the factors and pro's/con's involved.

Meanwhile, you have a framework of your own to work on and a community to build. Focus there, I'd say! ;-)

@robclancy
Copy link
Contributor

Not the issue won't come up at the conference unless we laugh at it. I'll slap anyone that takes it serious.

@thasmo
Copy link

thasmo commented Mar 8, 2014

Does this mean Laravel uses parts of my house? Btw. we shouldn't call them Surrogate, Bruce Willis is going to destroy them in the future. http://www.imdb.com/title/tt0986263/

@Anahkiasen
Copy link
Contributor

Come brothers, @pmjones has shown us the light of the way.

@cordoval
Copy link

cordoval commented Mar 8, 2014

I just hope for the sake of laravel reputation that they come to senses whenever. @pmjones is an old saga but he shows he never stops learning. Sad that some chose to otherwise.

@joshmanders
Copy link
Contributor

Best I can do, I'm a programmer not a photoshop wizard.

Taylor Surrogates

@robclancy
Copy link
Contributor

TIL more: I am not learning because the word I use correctly I don't use the same as someone else wants to use it because he thinks it has one meaning and thus cannot be used for anything else.

I must have stopped learning years ago. Thanks for the heads up @cordoval

@stormpat
Copy link

stormpat commented Mar 8, 2014

Come on Bennet, lets party!

@adamwathan
Copy link
Contributor

I built an address book once, it had a State class. Design pattern police shut it down real fast.

@AmyStephen
Copy link

Anyway, I've made my point. You guys want to insist on being wrong, fine. I've done what I can to help you out, and hopefully help my future self. If you are actively resistant to being shown the better way, and having it actually worked out for you, there's nothing else I can do.

@pmjones This is just a way for you to show the Laravel community will reject your PR? You knew it wouldn' t be accepted so you invested time creating the PR for the sole purpose of showing it be rejected? I'm at a loss. All that does is create ill will and wastes time.

@pmjones
Copy link
Author

pmjones commented Mar 9, 2014

No, I created because I want to give @taylorotwell the benefit of the doubt. I didn't invest three hours for it not to go through, but to show it could be done, and in a backwards-compatible way. The ball is in his court; if he wants to reject it out-of-hand, it's up to him. At this point I have done what I can.

@AmyStephen
Copy link

@pmjones Well, you knew he was not going to take it because he said he would only entertain documentation changes at this time, previous to you creating it. You are exceptionally bright, so, I think you did invest the time knowing it would not go through. Best to stop now. Allow the community time to consider the term usage themselves and decide what's best. Extend a little grace, please.

@robclancy
Copy link
Contributor

The community doesn't decide anything. Taylor does. And he has already, multiple times, decided on the issue (well the non issue, since it is being used correctly). And even said it wasn't going to happen before this PR was done. And 3 hours? Really? You have to be master troll. I bow to you. Teach me your ways.

@AmyStephen
Copy link

@robclancy You do not help the Laravel community with the way you act. Many times, people have pointed at your comments and blamed Taylor. Stop.

@robclancy
Copy link
Contributor

I think I won't listen to you. You only show up for drama anyway. Much like @pmjones is only here to start it to get more eyes on his stuff. Then brandon ebooks site to make more drama go there. All these hidden agendas. Then there is me. Just enjoying idiots :)

@AmyStephen
Copy link

@robclancy I don't think you listen to anyone and it creates problems for those you are trying to protect whether you stick your fingers in your ears and hum or you hear it. Whether the community accepts @pmjones idea, or not, is completely within the right of the community. No one needs to force their ideas on anyone but no one needs to mock someone sharing, either.

@Garbee
Copy link
Contributor

Garbee commented Mar 10, 2014

Two though processes here:

First, argumentative:

Using the provided definition for facade from @pmjones :

Provide a unified interface to a set of interfaces in a subsystem. Façade defines a higher-level interface that makes the subsystem easier to use.

Let's say I decide to build a shiny new Stripe package since I don't like the one they provide. I will need to use the HTTP API that they provide. So, now what? I don't want to work with curl directly, so I pull in Guzzle to handle that. So I hack away for a day or two and build a package, and yes, this includes a facade for Laravel.

What did I just do? I created a class that performs a task. It has a well defined API to interact with while dealing with another messier one (in this case, even Guzzle is a Facade since it is a nice API to interact with curl.) Does this situation not fit the definition of a Facade? You have a subsystem (in this case Guzzle and curl) which you build a class to interact with neatly. So where does this fail to be a facade?


Second, general not giving a crap:

So, let's just say without arguing, that; Yes, you are absolutely correct in Laravel's use of the term Facade is wrong. So what?

We call AJAX what it is, but we interact with it primary through something called an XMLHttpRequest. Why is this? Well, back when it was being done, someone in charge of IE had a thing for XML, so if that was in the name, it was more likely to get done. (While this exact thing is a bit more intricate that what information is provided here, the base point should still get across.)

Base point, things change based on what developers decide. Who says Facade was really properly defined 100% at its creation? Who says what Laravel is doing is not a facade in any fashion? Semantics change over time, it is humans that give words their definitions. Just as we change what booty means from being a term for a treasure stash back a few hundred years ago to meaning an ass in modern tongue, the definitions of software architecture can change over time. We make mistakes and we also evolve.

You are saying the current definitions are absolutely solid and should be respected exactly. Well, I say no. Laravel is now appearing to be the largest PHP project. And you are saying somehow one of its fundamental features is absolutely wrong. Well, if it is, obviously no one cares. If you think it is such an appalling mistake, then do it better. Show exactly how it is wrong. build a system, show how it is done the "right" way, and point out the differences. Then let developers decide based on that on their own if Laravel is wrong or not. You saying it is wrong, doesn't make it so. "Ain't" is technically wrong, yet it has a general understanding as to what it means. So should we smack everyone who uses the word to get them to stop?


There is a choice here. You can accept that we are imperfect beings. That means, Laravel's use of Facades is either proper now, or can one day in the future be proper. (The way I see it, the general developer audience seems to believe it is now based on discussions I have had with people in IRC.) Or, you can stand your morals absolutely. This means every Laravel developer doesn't know what they are doing, and you will never hire or recommend them. You know what? This is fine. That is a choice you can make, whether it will be good for your business or not, who knows.

The way I see it, Laravel is where it is because it is easy to understand. The API's are simple to understand and above all the terminology makes sense to the majority of people. There is no reason for a change like this now. Either Laravel will prosper even more and the definition can be improved or the general dev vocabulary will change, screw the definition.

@pmjones
Copy link
Author

pmjones commented Mar 10, 2014

And you are saying somehow one of its fundamental features is absolutely wrong.

Incorrect. I am saying that the behavior it implements, a proxy-like behavior, is named the wrong thing for how it actually behaves. A better name exists, one that implies the pattern actually being followed by the underlying code.

@joshmanders
Copy link
Contributor

@Garbee 👍

@Garbee
Copy link
Contributor

Garbee commented Mar 10, 2014

@pmjones, If I wrote that much and all you can pull out is that to argue with, then I do believe we can all see how pointless this is now.

@pmjones
Copy link
Author

pmjones commented Mar 10, 2014

It shows me you have a fundamental misunderstanding of the argument. The rest of it is fluff.

@joshmanders
Copy link
Contributor

Interesting. Anyone who disagrees with @pmjones has a fundamental misunderstanding of the argument. Anyone who agrees is correct.

@taylorotwell
Copy link
Member

gtfo

@AmyStephen
Copy link

The way I see it, Laravel is where it is because it is easy to understand. The API's are simple to understand and above all the terminology makes sense to the majority of people.

That's quite a complement, @Garbee

@GrahamCampbell
Copy link
Member

How is this still going on? Leave it already!

@pmjones
Copy link
Author

pmjones commented Mar 10, 2014

Feel free to leave it, as you wish.

@robclancy
Copy link
Contributor

No more comments after this one. @pmjones just wants attention. I think we have given him enough for at least a week of peace from him. Until the next semantic.

@pmjones
Copy link
Author

pmjones commented Mar 10, 2014

@robclancy +1 In fact, no comments from anyone except @taylorotwell who is the actual decision maker.

@GrahamCampbell
Copy link
Member

@pmjones You just made a comment...

@pmjones
Copy link
Author

pmjones commented Mar 10, 2014

Even when I agree, you're unhappy.

@laravel laravel locked and limited conversation to collaborators Sep 26, 2014
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