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

Allows users to disableDependencyChecker from .ember-cli options. #7413

Conversation

MiguelMadero
Copy link
Contributor

Right now we need to pass this option via the constructor,
which is used for tests, but as a user I don't have a way to disable this.
There are reasons to disable this outside of tests:

  • The DependencyChecker doesn't work today with yarn workspaces
  • We may be using a different way to check dependencies like yarn --check-integrity

Right now we need to pass this option via the constructor, 
which is used for tests, but as a user I don't have a way to disable this. 
There are reasons to disable this outside of tests: 
* The DependencyChecker doesn't work today with yarn workspaces
* We may be using a different way to check dependencies like yarn --check-integrity
@rwjblue
Copy link
Member

rwjblue commented Nov 2, 2017

You should either remove the addon or disable it via the SKIP_DEPENDENCY_CHECKER environment variable. Most likely the former if you are trying to always skip it...

@rwjblue
Copy link
Member

rwjblue commented Nov 2, 2017

Both of the bullets mentioned here infer that you just don’t need the addon. Removing it (just like any other addon) is a perfectly viable option and seems much more inline with what your actual intentions are here...

@MiguelMadero
Copy link
Contributor Author

That sounds like a good option. But I'm not sure what addon you're referring to.

I don't have ember-cli-dependency-checker installed. Not sure if that's the addon you're referring to. The problem seems to be on Project#hasDependency https://github.com/ember-cli/ember-cli/blob/mmadero/disableDependencyChecker-support/lib/models/project.js#130

@MiguelMadero
Copy link
Contributor Author

Looks like there is a separate bug introduced on #7401 I'll leave this open since it might make sense on its own

@MiguelMadero
Copy link
Contributor Author

I fixed it on #7414

@MiguelMadero
Copy link
Contributor Author

MiguelMadero commented Nov 15, 2017

@rwjblue

My main use case is addressed now by #7414. There may still be a use case for this flag. What do you think? If there is I can document it and merge this if not, I'm happy to just close the PR.

@MiguelMadero
Copy link
Contributor Author

@rwjblue ping!

@Turbo87
Copy link
Member

Turbo87 commented Feb 16, 2019

sounds like this is mostly resolved now :)

@Turbo87 Turbo87 closed this Feb 16, 2019
@MiguelMadero
Copy link
Contributor Author

👍

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

3 participants