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

Bugfix: adding deprecations won't throw on multiple installed copies … #183

Merged
merged 1 commit into from
Jun 10, 2019
Merged

Bugfix: adding deprecations won't throw on multiple installed copies … #183

merged 1 commit into from
Jun 10, 2019

Conversation

jenweber
Copy link

@jenweber jenweber commented May 17, 2019

This work should target 3.0.0 as a patch release candidate. It should not be merged to master, since some of the code is removed in unreleased #154 (a major release candidate).

Why this is needed

If two copies of ember-inflector are running, defineProperty will throw since the prototype is already defined. Passing {configurable: true} will allow it to get rewritten and therefore not throw. This will help people who are using local yarn links. See #182 for details.

Closes #182

@ef4
Copy link

ef4 commented May 18, 2019

The symptom you get if you hit this problem is the exception Cannot redefine property: pluralize.

I am in favor of releasing this fix as 3.0.1, assuming that the changes currently on master aren't slated to be released without a major bump.

@ef4
Copy link

ef4 commented May 31, 2019

@stefanpenner or @rwjblue: are you OK with merging this into a new non-master branch and releasing this as 3.0.1? I can handle that if you give me the bits.

It wouldn't go into master because master has already removed this deprecated code, but master is unreleased and presumably will be a semver major.

@rwjblue
Copy link
Member

rwjblue commented Jun 3, 2019

@ef4 - Seems good, I think I've fixed the permissions.

@stefanpenner
Copy link
Member

I'm fine with changing this, but we should likely warn or something if someone installs two copies of this lib or ember right?

@ef4 ef4 changed the base branch from master to stable-3.x June 10, 2019 02:26
@ef4
Copy link

ef4 commented Jun 10, 2019

I don't think it's a concern of this library to warn about duplicate deps. That is a bigger issue. We need general-purpose linting for that, and we need to give people something actionable to do about the warnings when they get them.

@ef4 ef4 merged commit 0402a58 into emberjs:stable-3.x Jun 10, 2019
@ef4
Copy link

ef4 commented Jun 10, 2019

@rwjblue this is merged into stable-3.x and I tagged a 3.0.1 release there, but I don't have bits on NPM to publish it.

@stefanpenner
Copy link
Member

@ef4 you should now have the power:

npm owner add ef4 ember-inflector
+ ef4 (ember-inflector)

@sandstrom
Copy link
Contributor

@ef4 Would you like to publish the 3.0.1 release?

I've updated the changelog here:
#281

@ef4
Copy link

ef4 commented Jun 16, 2020

I published 3.0.1 almost a year ago.

That is separate from the question of the still-unreleased breaking changes on master. Those should be released as 4.0.0, but I wasn't at all involved in them and don't know what other steps should be taken before releasing.

@sandstrom
Copy link
Contributor

@ef4 Ah, my bad!

It's this commit that I would like to get access to: 4a0d99c

Those deprecations are marked as "until: 3.0.0" so I'm thinking that they don't really need to be considered breaking. But you know these policies better than I do.

Anyway, a 3.0.2 or 4.0.0 with the changes from master would be awesome! Right now we're running off a private fork.

maybe @jenweber or @rwjblue can chime in?


Entirely separate, but if you have commit bit to this repo, perhaps we could close a bunch of old issues, that aren't relevant anymore (let people re-open if needed).

https://github.com/emberjs/ember-inflector/issues

@sandstrom sandstrom mentioned this pull request Jun 23, 2020
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.

None yet

5 participants