taptapship / wiredep

Wire Bower dependencies to your source code.
MIT License
1.15k stars 142 forks source link

Fix issue 107 #137

Closed dnozay closed 9 years ago

dnozay commented 9 years ago

:)

stephenplusplus commented 9 years ago

Thanks for this, but I don't believe this is a fix for the issue. A user who doesn't know they have to run bower install before using wiredep won't know to handle the error. If they did make the effort to handle the error, the error wouldn't occur in the first place!

I think throwing an error with a more helpful message, like: "name of component was not found. Make sure to run bower install before using wiredep."

dnozay commented 9 years ago

Thanks for this, but I don't believe this is a fix for the issue. A user who doesn't know they have to run bower install before using wiredep won't know to handle the error. If they did make the effort to handle the error, the error wouldn't occur in the first place!

I usually run in this issue (often) when I rebase my code and someone added extra deps and I forget to run bower install.

stephenplusplus commented 9 years ago

That's more of a local workflow issue, not something that needs to be solved by the tool you're using. I forget to run pre-commands like this all the time, and because I've done it before, I remember what to do to fix it. I don't want to allow a developer to handle this error - it is an error that should be thrown because wiredep cannot proceed.

dnozay commented 9 years ago

@stephenplusplus I am not going to argue with you. This fix gives a more user-friendly message. If you believe the user is at fault for having a broken workflow; that's fine. But how can they fix the workflow when they don't know or understand why it is broken in the first place?

stephenplusplus commented 9 years ago

I agree that we want them to understand the problem, which is why I suggest a nicer error message than the "Error on line XX" one. I just don't think we should give them a built-in way to handle the error.

On Wednesday, December 10, 2014, Damien Nozay notifications@github.com wrote:

@stephenplusplus https://github.com/stephenplusplus I am not going to argue with you. This fix gives a more user-friendly message. If you believe the user is at fault for having a broken workflow; that's fine. But how can they fix the workflow when they don't know or understand why it is broken in the first place?

— Reply to this email directly or view it on GitHub https://github.com/taptapship/wiredep/pull/137#issuecomment-66517535.

dnozay commented 9 years ago

I just don't think we should give them a built-in way to handle the error.

some folks are using bower some folks are using higher-level tools; where a different error message may be more appropriate; the error handler is needed for unit tests.

stephenplusplus commented 9 years ago

Let's start with just giving a better error message, and if there's enough noise about a better solution, we can solve it at that time. Thanks for your help.

dnozay commented 9 years ago

@stephenplusplus, I think I have made the minimum-required change. How do you suggest I simplify the implementation? Can you please comment directly on the diffs? thank you.

stephenplusplus commented 9 years ago

Looks like my earlier self built in a generic on-error handler, so I went and used it for this new error condition: https://github.com/taptapship/wiredep/commit/8961229e1753d527738647e63f7b5e42ef72eb8c

released as 2.2.2