Nordstrom / artillery-plugin-aws-sigv4

Plugin for artillery.io that signs HTTP requests using the AWS Signature V4 specification.
Apache License 2.0
16 stars 21 forks source link

Gulp enabled and git commit hooked. #2

Closed gwsii closed 8 years ago

gwsii commented 8 years ago

Adds gulp for tooling support. Using guppy for git hooks, now runs lint and style tasks before commit is allowed.

gwsii commented 8 years ago

Think this is a good pattern for hooking these and enforcing basic JS code quality.

erikerikson commented 8 years ago

If we're going to do this and enforce it, we should hook the pre-commit in package.json

gwsii commented 8 years ago

Are you talking about the package.json scripts supported by npm? Is that what you mean? I'd agree hooking the prepublish event would be a great idea!

erikerikson commented 8 years ago

Yea, sorry to be vague. I was hoping to be more useful than that. Just wanted to sort out how to deal with cross platform but something like:

{
  "scripts": {
    "myScript": "..."
  }
  "pre-commit": [
    "myScript"
  ]
}
erikerikson commented 8 years ago

That's dependent upon the pre-commit module: https://github.com/dwyl/learn-pre-commit

I've gotta add this to the cloudwatch plugin too.

erikerikson commented 8 years ago

Actual module: https://www.npmjs.com/package/pre-commit

gwsii commented 8 years ago

Gotcha. So the main difference between the module you reference and the one I had selected is that guppy can be used to invoke gulp tasks; which I have also made the choice to use.

Both pre-commit and guppy-pre-commit use the same mechanism of the git commit hook. In the case of pre-commit, it will invoke a set of script actions as defined in the package.json file. With guppy, gulp actions are used. My experience so far is that npm script commands are hard to get right cross-platform, but node code running in a gulp context is hopefully more compatible.

Still need to determined if we need npm script commands to do things like code coverage or other stuff.

What do you think?

erikerikson commented 8 years ago

Those're good points.

I was intending to avoid being opinionated about gulp vs grunt vs ???. That plus gulp has the weird global-local install split. I say these being a fan of gulp. "pre-commit" is super light byte-per-byte.

These should be reasonable and sufficient for our purposes, no?

  "scripts": {
    "jscs": "jscs lib tests",
    "jshint": "jshint --exclude node_modules .",
    "test": "[TODO]"
  },

Probably worth a little time to find the good way of slicing this one without the pains that you mention.

gwsii commented 8 years ago

Cool. I'm going to re-branch and you should see an different PR shortly.