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

add compatibility with strict mode #665

Merged
merged 1 commit into from
Apr 6, 2016
Merged

Conversation

sukrosono
Copy link
Contributor

Base on my captain comment
CMIIW 😄

@sukrosono
Copy link
Contributor Author

Dear our lovely maintainer,

I am concern with the test coverage, regardless the debate of it.
Why? When I change the point of views as Chai user, I can get some reason to not use chai, since it's reflected on the badge on readme. It's just 69% right now.

Attempt to increase it is possible solution to gain Chai user trust.

A guidance and discussion to increase it bit by bit sounds good to me 😄

@keithamus
Copy link
Member

Great work @brutalcrozt!

As for the code coverage, this is something we will be working on soon - as we split out more of the codebase into modules, and each module gets full test coverage.

@keithamus
Copy link
Member

LGTM btw. @lucasfcosta?

@sukrosono
Copy link
Contributor Author

Why we break it into smaller modules? Beside it easy to organize?
I want to know before and after the v4.0.0, where i can see it?
sorry if i am too noisy, did it will reflect on the change log?

@keithamus
Copy link
Member

@brutalcrozt we're breaking it into smaller modules for many reasons, namely:

  • Maintainers can be added to smaller modules without such a large impact on the whole codebase
  • Smaller components can be given much more autonomy vs chai-at-large
  • Hopefully people might get use of the smaller modules, without having to use chai
  • It better fits npm philosophy of many small modules

We have a roadmap issue - #457 - which discusses this, and discusses what will parts will be moved out and when. You should have a look through this issue if you want to find out more.

@lucasfcosta
Copy link
Member

LGTM, thanks @brutalcrozt!

Regarding the test coverage, it really makes sense, thanks for pointing this out.

@lucasfcosta lucasfcosta merged commit 0c68b23 into chaijs:master Apr 6, 2016
@sukrosono
Copy link
Contributor Author

You're welcome. Is it good guys to create a issue which provide to do 📋 per next major release?

Don't forget to be happy 😄

try to spell the magic word fixes #578

Did i become a magician? :shipit:

@keithamus
Copy link
Member

You have to add fixes #578 to your original comment, or the commit message to make the magic happen. But I've closed it now so let's just pretend 😉

@keithamus
Copy link
Member

Also I think when I get some time to properly go through the issues, I'll set up milestones which will be much better at tracking major releases.

@sukrosono
Copy link
Contributor Author

You have to add fixes #578 to your original comment, or the commit message to make the magic happen. But I've closed it now so let's just pretend

Thanks, latter will do that on commit message. Since the magic happen when it merged

Also I think when I get some time to properly go through the issues, I'll set up milestones which will be much better at tracking major releases.

Got it captain 😄

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