Skip to content

Conversation

@revelt
Copy link

@revelt revelt commented May 10, 2017

Let's add some examples of how to run both AVA and NYC on watch mode. Also, I fixed few English grammar errors in the original copy.

… nodemon. Also, fixed few grammar errors in the original text.
Copy link
Member

@novemberborn novemberborn left a comment

Choose a reason for hiding this comment

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

Thanks for the grammar fixes 👍

This doesn't really integrate with AVA's watch mode though: nodemon will cause all tests to be rerun. That's useful of course if you want up-to-date coverage but I wouldn't recommend it for regular usage.

Perhaps this can be changed into more of a "Tip: Re-run nyc using nodemon" section? That could suggest installing nodemon globally and invoking nodemon --quiet --watch ./ --exec npm run coverage directly.

We may also want to update this recipe to use nyc's configuration options, to avoid having to specify various --reporter values in the nyc invocation.

### Use the Babel require hook

To use the Babel require hook, add `babel-core/register` to the `require` section of you AVA config in `package.json`.
To use the Babel `require` hook, add `babel-core/register` to the `require` section of you AVA config in `package.json`.
Copy link
Member

Choose a reason for hiding this comment

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

Babel just uses require hook.

While we're updating this though, we should recommend using babel-register, not babel-core/register which is undocumented (and, without verifying, might very well be going away in Babel 7).

Copy link
Author

Choose a reason for hiding this comment

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

Thank you. I extended the tick to include the word hook too. Let's do babel-register as a separate commit, let's get nodemon example sorted first. What do you think?


```json
"scripts": {
"coverage": "nyc report --reporter=text-lcov | coveralls",
Copy link
Member

Choose a reason for hiding this comment

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

This is unrelated to what you're trying to show.

Copy link
Author

Choose a reason for hiding this comment

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

I considered your point, slept over it and considered again. This line is called by .travis.yml. Without it, Travis won't be able to ping Coveralls. We can omit it, but then we should note aside that scripts snippet is incomplete. Some people might think it's complete, paste gung-ho and their Coveralls won't update. Being a lazy person, I like complete snippets, ones without missing bits.

I'm happy to remove this line, just say no and I'll remove it.

Copy link
Member

Choose a reason for hiding this comment

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

It's not used by .travis.yml. Read the above .travis.yml code.

```json
"scripts": {
"coverage": "nyc report --reporter=text-lcov | coveralls",
"test": "standard && nyc --reporter=html --reporter=text ava",
Copy link
Member

Choose a reason for hiding this comment

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

standard && too.

And put the reporter option as package.json config instead CLI flags. (See the nyc docs).

Copy link
Author

Choose a reason for hiding this comment

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

Done. Please review. I tested the separated setup on one of my libraries, seems fine on mine. Thank you, that's a new thing learned. 👍

"scripts": {
"coverage": "nyc report --reporter=text-lcov | coveralls",
"test": "standard && nyc --reporter=html --reporter=text ava",
"watch": "nodemon --quiet --watch ./ --exec npm run test"
Copy link
Member

Choose a reason for hiding this comment

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

./ => .

Copy link
Author

Choose a reason for hiding this comment

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

Done good spot. Please review.


First, install `nodemon` as a dev dependency:

```bash
Copy link
Member

Choose a reason for hiding this comment

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

bash => console

Copy link
Author

Choose a reason for hiding this comment

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

Done, good spot. I remember some of my editors had issues in the past recognising console but it's probably an old reflex. Thank you. Please review.


Your coverage report will then appear on coveralls shortly after Travis completes.

## Integrating with AVA's Watch mode
Copy link
Member

Choose a reason for hiding this comment

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

Watch => watch

Copy link
Author

Choose a reason for hiding this comment

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

Reworded the title completely. Like Mark noted, it was not accurate actually, we can't say "AVA's watch" because it's "nodemon's watch". Please review.

"watch": "nodemon --quiet --watch . --exec npm run test"
},
"nyc": {
"reporter": [
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect indentation here.

@revelt
Copy link
Author

revelt commented May 11, 2017

Hi Sindre, guys, I'll take down the PR, but feel free to amend, reuse or replace the text whichever way you want (or drop it completely).

@revelt revelt closed this May 11, 2017
@sindresorhus
Copy link
Member

@revelt Why?

@revelt
Copy link
Author

revelt commented May 11, 2017

@sindresorhus that's fine, feel free to fix it and publish. It will be more effective, less backwards forwards. Seriously, no hurt feelings. I'll help all I can and keep contributing.

@sindresorhus
Copy link
Member

No, I mean, why not just finish it? You only had a couple of super minor changes left. I'm curious, as I often see people just not finishing almost done PRs and I still don't understand why.

@revelt
Copy link
Author

revelt commented May 11, 2017

You know Monopoly, it was a popular game but its rules inadvertently alienated the families. It was people winning against people, not a family winning together against imaginary "Monopoly". I find GitHub a bit of a similar thing. Documentation editing on OS projects should be like a happy Google Docs editing session: somebody writes up something new, others jump in and everybody edit the damn thing TOGETHER, quickly, happily and in live mode. Like a public Google Doc. If admin/maintainer does not like certain character, he just edits it over and happy days. As opposed to formal "requests" and rubbing in to original poster's ego (no offence, I'm talking in general). I know I was not entirely exact on some places, but "requests" to amend make it public which hurts. It's like Monopoly, giving away your albeit imaginary money and failing in front of siblings. Also, you know, any change, even to readme, triggers CI on GitHub. They should review the whole documentation concept a bit - how people work together.

@sindresorhus
Copy link
Member

It was people winning against people, not a family winning together against imaginary "Monopoly".

But open-source is "a family winning together"! 🤗

As opposed to formal "requests" and rubbing in to original poster's ego

I find it 😔 that you consider a PR review "rubbing it in". Rather look at it as an opportunity to learn or see other ways to solve problems. Your PR was actually correct, it was just not according to our preferences and required some minor tweaks, which you couldn't have known, and it's as expected. Very few, or close to no PRs are merged without changes.

I could have done the changes myself of course. It would have taken much less time. But I value pull request reviews, both as a submitter and as a reviewer. I often learn things as a reviewer on PRs. I suggest some change and the submitter elaborates on why the existing way is better. I don't always request the best changes, but by requesting something, I start a conversation about it.

I would recommend leaving out "ego" and "emotions" when doing open-source. Yes, I know it's hard.

@StoneCypher
Copy link
Contributor

linters often prevent hurt feelings over formatting rules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants