search

poteto / ember-changeset-validations

Validations for ember-changeset
http://bit.ly/ember-changeset-demo
Other
220 stars 98 forks source link

Move `ember-changeset` to `devDependencies` #338

Closed SergeAstapov closed 2 years ago

SergeAstapov commented 2 years ago

Closes # .

Changes proposed in this pull request

snewcomer commented 2 years ago

Is this to allow the consumer to put in their own?

josemarluedke commented 2 years ago

This is a nice change. It seems we don't import ember-changeset in the addon itself, just in the dummy app.

SergeAstapov commented 2 years ago

@snewcomer correct! As @josemarluedke said the ember-changeset-validations addon itself does not depend on ember-changeset, only in tests.

For the consuming app, there is not much value to bring this addon without ember-changeset but that should be responsibility of the app author to use or not use ember-changeset.

IMO this makes things crystal clear about "who does what".

May need to be have "breaking" label as technically, if someone did not install ember-changeset themselves their app may break.

SergeAstapov commented 2 years ago

build failure is the same as we have in master. Need to land #335 to fix CI and then I'll rebase this PR.

SergeAstapov commented 2 years ago

@snewcomer this is ready to go! cc @josemarluedke

snewcomer commented 2 years ago

Probably some copyright to add to the README to indicate ember-changeset also needs to be installed along side this add-on?

SergeAstapov commented 2 years ago

@snewcomer done! Thanks for idea