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

getPretender and setPretender functions as public exports #338

Merged
merged 2 commits into from
Apr 28, 2018

Conversation

ryedeer
Copy link
Contributor

@ryedeer ryedeer commented Apr 25, 2018

Hello! This is the PR I promised to make some time ago. It allows to import the getPretender function to use the FactoryGuy's instance of Pretender for other tasks. Also it includes the setPretender function to let the FactoryGuy use the Pretender instance created by some other library.

@danielspaniel
Copy link
Collaborator

Hi @ryedeer , remind me again why you/someone would want to setPretender ( can you think of a use case for that ? ) I am having trouble. I guess i forgot our last conversation.

@ryedeer
Copy link
Contributor Author

ryedeer commented Apr 27, 2018

The reason to have setPretender: only one instance of Pretenter is allowed to exist, and some third-party library that also uses Pretender can also instantiate it, so it would be nice to have a mechanism to use the instance that already exists instead of trying to create another one.

@danielspaniel
Copy link
Collaborator

RIght, I understand that Oleg, I am just trying to think of a case where it would actually be used? Would you for example use setPretender?

@ryedeer
Copy link
Contributor Author

ryedeer commented Apr 27, 2018

Currently, we only use getPretender, so I cannot show a specific use case for setPretender. Well, I can just delete all traces of it from the pull request... It wouldn't be hard to implement it when it is really necessary.

@danielspaniel
Copy link
Collaborator

I am not trying to be difficult and spoil a nice PR Oleg, I just want to make sure to only add things that actually would be useful, and not create things that no one would really use. So if you can remove that setPretender till you need it or someone asks for it that would make me happier.

@ryedeer
Copy link
Contributor Author

ryedeer commented Apr 28, 2018

Yes, I understand it. Here's a commit that removes setPretender.

Unrelated to this PR: may I ask you to check my recent comment to the issue 231? A similar problem exists again, and I cannot reopen that issue because it wasn't closed by me.

@danielspaniel danielspaniel merged commit 165edc3 into adopted-ember-addons:master Apr 28, 2018
@ryedeer ryedeer deleted the public_pretender branch June 18, 2019 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants