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

Updates and improvements #12

Closed
wants to merge 6 commits into from
Closed

Updates and improvements #12

wants to merge 6 commits into from

Conversation

lagden
Copy link

@lagden lagden commented Feb 23, 2018

  • change test framework mocha -> ava and istanbul -> nyc
  • prefer async/await
  • add xo lint (eslint)
  • travis run nodejs 8 LTS

- prefer async/await
- change mocha -> ava
* update:
  Upgrade and improvements
@coveralls
Copy link

coveralls commented Feb 23, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 3524318 on lagden:master into 4b19eb1 on koajs:master.

@haoxins
Copy link
Member

haoxins commented Feb 27, 2018

change test framework mocha -> ava and istanbul -> nyc

why ?

@lagden
Copy link
Author

lagden commented Feb 27, 2018

I didn't know about eslint-config-koa...
added!

why?

Mocha requires you to use implicit globals like describe and it with the default interface (which most people use). It's not very opinionated and executes tests serially without process isolation, making it slow.

@jonathanong
Copy link
Member

generally styling fixes are not accepted. i would take the .travis.yml updates though

@jonathanong
Copy link
Member

and @coderhaoxin 's suggestions

@lagden
Copy link
Author

lagden commented Mar 2, 2018

@jonathanong

the style is according with the eslint-config-koa

https://github.com/lagden/conditional-get/blob/3524318c7c854efd479b75bd2fac182c7388029e/package.json#L41

and mocha -> ava was "upgrade", but the test content is the same...

Any further suggestions?

@haoxins
Copy link
Member

haoxins commented Mar 22, 2018

#13

@haoxins haoxins closed this Mar 22, 2018
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.

None yet

4 participants