Skip to content
This repository was archived by the owner on Jan 30, 2023. It is now read-only.

Conversation

@gnapse
Copy link
Contributor

@gnapse gnapse commented Jan 16, 2017

The options object could be frozen and therefore not modifiable. It is safer to work with a clone of it. This PR does just that.

If for some reason it is not acceptable to assign to the options argument, let me know and I can modify this accordingly.

The `options` object could be frozen and therefore not modifiable. It is safer to work with a clone of it.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.942% when pulling fe11561 on gnapse:patch-1 into 7d2a951 on ember-cli:master.

@alexlafroscia alexlafroscia merged commit 8e1f050 into ember-cli:master Jan 16, 2017
@alexlafroscia
Copy link
Collaborator

Great points, thanks @gnapse!

@alexlafroscia
Copy link
Collaborator

Hmm, I know I already merged it but would you mind adding a test case for this in a separate PR @gnapse?

@gnapse
Copy link
Contributor Author

gnapse commented Jan 17, 2017

Sure @alexlafroscia, I just did. Check out #227.

@gnapse gnapse deleted the patch-1 branch January 17, 2017 19:07
alexlafroscia pushed a commit that referenced this pull request Jan 17, 2017
alexlafroscia pushed a commit that referenced this pull request Feb 15, 2017
* Clone options object before modifying it

The `options` object could be frozen and therefore not modifiable. It is safer to work with a clone of it.

* Avoid accessing Ember.merge directly
alexlafroscia pushed a commit that referenced this pull request Feb 15, 2017
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.

3 participants