[GitHub] brodybits edited a comment on issue #377: Increase Code Coverage & use nyc

[GitBox <gi...@apache.org>]

brodybits edited a comment on issue #377: Increase Code Coverage & use nyc
URL: https://github.com/apache/cordova-ios/pull/377#issuecomment-410573590
 
 
   @erisu now I understand a few things. My one major criticism is that I think `npm test` should do all existing tests, meaning `npm test` should include e2e (general case) or component (this case). If user does not have standard iOS dev tools including ios-deploy and pod then I think it is fine to say that full `npm test` would not work. I think these tools should not be extremely hard to install for someone working on cordova-ios.
   
   I personally do not really like squashing multiple kinds of changes together, like both migrating to nyc and adding more test coverage. More things at the same time could be more to grasp at the same time. Major factor is that if someone wants to back-port to an earlier release, separate commits would be nicer. But this is just my personal opinion, some others seem more likely to squash.
   
   ~~And a side question: I thought you had made a change to introduce jasmine-spec-reporter but couldn't find it. Maybe it was someone else. I think it would be nice, probably in a separate PR.~~ __Update:__ _It was someone else who introduced jasmine-spec-reporter on cordova-lib, which is now merged, would make a nice separate PR on this and other Cordova repos._

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org