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

emit errors if hit any circular dependency #7

Merged
merged 1 commit into from
Feb 5, 2015
Merged

emit errors if hit any circular dependency #7

merged 1 commit into from
Feb 5, 2015

Conversation

huang47
Copy link

@huang47 huang47 commented Feb 4, 2015

To address issue #6

backflip added a commit that referenced this pull request Feb 5, 2015
Emit error when detecting circular dependency
@backflip backflip merged commit eb5cd50 into backflip:master Feb 5, 2015
@backflip
Copy link
Owner

backflip commented Feb 5, 2015

Awesome, thanks!

@backflip
Copy link
Owner

backflip commented Feb 5, 2015

On second thought: Should we treat this as a major change (v2.0.0) since it might break existing implementations (which for some reason rely on circular dependencies being silently ignored)?

@huang47
Copy link
Author

huang47 commented Feb 5, 2015

good point, how about making it configurable to be backward compatible ?

config.checkCircularDependency = true | false ?

@backflip
Copy link
Owner

backflip commented Feb 5, 2015

Good idea.
I went with the following approach:

  • 1.1.0 adds a new option ignoreCircularDependencies which defaults to true (still using the old behavior)
  • 2.0.0 switches its default to false (emits an error)

@huang47
Copy link
Author

huang47 commented Feb 5, 2015

perfect (y)

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.

2 participants