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

Allow for clearing of require values through streams as per #10 #11

Merged
merged 1 commit into from
Mar 27, 2015
Merged

Allow for clearing of require values through streams as per #10 #11

merged 1 commit into from
Mar 27, 2015

Conversation

JesseFarebro
Copy link
Contributor

Couldn't figure out the test coverage branch that got deducted, but here is an initial pull request. If you want any changes let me know

I was also going to write a test case for resetting the file module through a commonjs stream, you only have an AMD require being reset through the stream right now. Let me know

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 28e4da4 on JesseFarebro:clear-requires into 1ecd02d on wbyoung:master.

@wbyoung
Copy link
Owner

wbyoung commented Mar 23, 2015

@JesseFarebro this looks good. Can you switch to === and squash to one commit?

I don't know what you mean by test coverage branch that got deducted, so if that was important, can you elaborate?

@JesseFarebro
Copy link
Contributor Author

The coverage summary,

Branches: 83.33% 20/24

I've never worked with coveralls or code coverage before so I'm not sure. I've updated the pull request with your request.
Thanks :)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e8cc0ad on JesseFarebro:clear-requires into 1ecd02d on wbyoung:master.

wbyoung added a commit that referenced this pull request Mar 27, 2015
Allow for clearing of require values through streams as per #10
@wbyoung wbyoung merged commit 788f71e into wbyoung:master Mar 27, 2015
@wbyoung
Copy link
Owner

wbyoung commented Mar 27, 2015

Thanks… not too concerned with 100% code coverage of branches at this point, but if the project grows, it might be worth the effort. v0.1.2 has been published with this change included.

@JesseFarebro JesseFarebro deleted the clear-requires branch March 28, 2015 18:39
@JesseFarebro JesseFarebro restored the clear-requires branch March 29, 2015 19:27
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.

3 participants