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

A few devops ideas. #374

Closed
ariporad opened this issue Dec 26, 2015 · 16 comments
Closed

A few devops ideas. #374

ariporad opened this issue Dec 26, 2015 · 16 comments
Assignees
Labels
enhancement new functionality

Comments

@ariporad
Copy link
Contributor

Hi All,

Over the past few days, I've been doing a bit of work on AVA, and I had a few ideas to improve the developer experience, and I wanted to get some feedback on them:

  1. babel.js is really poorly named. (Personal pet-peeve).
  2. It would be nice if there was a way to run tests without coverage, something like npm run test-fast. Tests are way faster without coverage.
  3. A watch mode would be nice too.

Thoughts?

@sindresorhus
Copy link
Member

  1. Any suggestions for what it should be named?
  2. Yeah, let's just do that by default and put the coverage in a separate run script and update .travis.yml
  3. Watch support #70

@sindresorhus sindresorhus added the enhancement new functionality label Dec 26, 2015
@jamestalmage
Copy link
Contributor

  1. 👍
  2. The penalty for coverage is about to drop from 17 seconds to 3 seconds Caching 2.0 istanbuljs/nyc#108. I don't think that is necessarily worth maintaining an additional build path.
  3. Watch support #70 is adding the feature to AVA, I think @ariporad means he wants a good way to use a file watch while hacking on AVA. That would be a tap feature request.

More on 2:

Even without coverage: 20-25 seconds is still a really long time to wait for a failure.

Really the best thing to do is learn the tap cli. I definitely did not fully understand it when I started hacking on AVA, and I think I even proposed something similar to your ideas here at some point. I (like @ariporad I believe) was most accustomed to testing in mocha, so there was a definite learning curve.

When I am hacking on a particular AVA problem, and want to focus on a particular file (let's just say fork.js), this is the command I use:

tap -b test/fork.js

The -b is short for --bail-out, which makes tap exit as soon as it sees a single failure. Usually that is enough to quickly identify a problem you can fix and move on.

@ariporad
Copy link
Contributor Author

@jamestalmage, Thanks, thats a good tip to know! But I still think that we don't need to run coverage by default. And I did mean watch for AVA itself. I was just thinking nodemon as a dev dependency.

@sindresorhus: child.js?

Can I go ahead and implement #70? It seems no one has done so.

@sindresorhus
Copy link
Member

child.js?

👍

Can I go ahead and implement #70? It seems no one has done so.

I'm not sure I want to deal with that right now. We have enough Babel related overhead and issues. Watching comes with its own headaches. The watch functionality in Node.js core is unreliable and the various watcher libs have edge-cases and downsides, like requiring native addons, which is a no go for us. We've added a lot of features lately, and while I realize it's more fun adding features than fixing bugs, I think we should prioritize the latter right now. My top priority is stabilizing the core and APIs, fixing bugs, and improving performance.

@ariporad
Copy link
Contributor Author

@sindresorhus: Ok.

@ariporad
Copy link
Contributor Author

Oh, @sindresorhus: Should I implement a coverage-less test option and a npm run watch too?

@sindresorhus
Copy link
Member

Should I implement a coverage-less test option

Actually, we already have test-win for that. We could just rename it into something more generic. (and update appveyor.yml)

npm run watch too?

👍 from me. @jamestalmage ?

@ariporad
Copy link
Contributor Author

@sindresorhus: How does test sound, and we can rename the test with coverage test:cov?

@ariporad ariporad self-assigned this Dec 27, 2015
@jamestalmage
Copy link
Contributor

What did you have in mind for a watch script?

@sindresorhus
Copy link
Member

@jamestalmage Do you have any preference on having coverage by default when running npm test? I never use it personally so wouldn't mind moving it into test:cov, but don't have any strong opinion about it.

@jamestalmage
Copy link
Contributor

Well, when I have 100% coverage - yeah, I like having it there so I know when I'm introducing new code that isn't covered. Since we are so far off complete coverage, it's really hard to see exactly where you are dropping coverage, I guess we could turn it off.

@ariporad
Copy link
Contributor Author

@sindresorhus: I have one more larger idea, which I'm not sure if you've ever considered: What about adopting the Angular commit message guidelines? They make it really easy to see what a commit is about, and it allows you to do cool things, like semantic-release (which could be really helpful for some of your smaller projects). I use them for all my projects, and it's really nice. (I'm sure you've probably seen it, but I thought I'd suggest it. If for whatever reason you don't want to implement it, that's 110% fine).

@sindresorhus
Copy link
Member

@ariporad It's a good idea, but too annoying in reality. Nobody reads the contributing guidelines, so you end up having to tell people (often multiple times) how to format the commit. It creates annoyance for both the contributor and maintainer with minimal gain. I've seen this so many times in the wild. As for semantic-release, I'm not convinced.

That being said, I would be happy to improve on our commit messages, but don't want to enforce it on contributors.

@ariporad
Copy link
Contributor Author

@sindresorhus: Fair enough. (Although if you ever change your mind, GitCop can do that automagically).

@sotojuan
Copy link
Contributor

semantic-release looks awesome and I am personally going to use it in my projects but at this point switching to it would be more pain than it's worth. I think Sindre and the team are going to make sure the version numbers stay sane. Same with enforcing commit message guidelines (even with tools like commitizen).

On another note, I admit that whenever I am working on AVA I delete the coverage option in the test script, which means one of these days I am going to forget it to put it back before committing. A regular test script would be great!

@novemberborn
Copy link
Member

Closing this since the initial issues have been addressed in AVA or are issues with third-party dependencies. Future suggestions on how we can make AVA easier for you to hack on are most welcome though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new functionality
Projects
None yet
Development

No branches or pull requests

5 participants