Skip to content
This repository has been archived by the owner on Oct 19, 2023. It is now read-only.

Add precompiler/compiler subargs options w/ tests #31

Merged
merged 3 commits into from
Aug 25, 2014

Conversation

knownasilya
Copy link
Contributor

Let me know if anything is out of place, this is the subargs version of #25

try {
precompiler = require(opts.p || opts.precompiler);
} catch (e) {
throw e;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of this try-catch wrapper as it throws anyway? @knownasilya

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@epeli good point. Would you suggest removing it or adding a more focused error message? Something like 'couldn't require the precompiler: error here'

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just remove the wrapper completely and let internal error to throw? Is it too ambiguous? i.e. I'm in favor of removing it if it does not have a good reason to be there :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok agreed 👍 if someone thinks it's too ambiguous after the fact they can submit a PR or issue.

@knownasilya
Copy link
Contributor Author

Try/catch removed.

@esamattis esamattis merged commit 888925d into esamattis:master Aug 25, 2014
@esamattis
Copy link
Owner

Merged and released to npm. Thanks for you patience with me 👍

@knownasilya
Copy link
Contributor Author

No problem, I know how difficult it can be to maintain consistency 😉 Thanks for the quick release!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants