Repository structure

[blakeembrey]

A couple of minor (subjective) things that would go a bit to helping out others (like me) get started committing.

• Rename bin/ to scripts/, node modules normally reserve bin/ for CLIs that the module exposes
• Use pre-commit node module and remove bin/git-hook - it would do very similar things in the end, except the pre-commit module just works automatically (by default it'll run the tests, but you can change that if you only want to compile templates - I usually do both in my test script and I made that change with Latest schema v4 #33)
• Make aliases for other scripts in package.json "scripts" - makes script discovery much easier and you can probably remove half the one liners you have in bin/ right now in favour of npm run bundle, npm run pretest, etc.
• Super subjective. Look at using something like standard for linting
• Make linting run as part of the test suite so PRs match your style
• Add karma tests to the npm test script - I only just discovered that it's even an option and I've glanced over it a dozen times

I can make a PR with these options enabled/disabled - just let me know which you don't want.

[epoberezkin]

I removed npm run build from npm test because tests (especially in travis) should run against compiled dot files in the repo rather then recompile them - many users are likely not to run tests / recompile dot files.

I think linting in script is a good idea - it will prevent some code errors from being comitted - at the moment it only happens in code-climate and there is a file that turns off the rules I intentionally break.

I don't like standard though - although with some things it enforces I agree, it's opinionated differently from me :) and not configurable. I disagree about not using semicolons - not only consistently using them prevents some errors but also, for example, }; vs } helps understand what this brace belongs to. I also disagree about always using '===' - I have many arguments for always using '==' unless you know that you definitely need '==='. I almost never use braces around single statements after if and for - for example, I prefer for (var key in schema) if (rules[key]) return true; to for (var key in schema) { if (rules[key]) { return true; } }... I know that many people think differently and quote Crockford. I prefer Flanagan's approach to JS :)

So I'm happy with any tool as long as it supports options from .jshintrc.

[blakeembrey]

@epoberezkin Fair enough, not sure I agree. Normally I'd avoid checking in any compiled files though and I'd just publish them using the npm prepublish step.

I'll look into a linting solution later for you. Shouldn't be difficult just to use jshint and all the newer projects tend to use eslint now too.

[epoberezkin]

Removing them completely is ok, I think. I didn't understand that it is what you suggest as they were still there. It will guarantee they are not old.

It also makes tracking changes a bit easier too. I will do it.

jshint is ok, definitely.

[blakeembrey]

It's ok, I don't think I had made that suggestion yet. But yeah, I normally just prepublish and run it on build, and keep compiled pieces out of the source code.

[solsson]

@epoberezkin It looks like the current build in npm, 1.1.0, is broken, at least for use with webpack. I got errors like Error: Cannot resolve 'file' or 'directory' ../dotjs... when bundling. Fixed through cd node_modules//ajv/ && npm install. I guess the prepublish script should have done this but instead the dotjs folder had only the readme.

[blakeembrey]

Can confirm. This is a result of, when you don't specify the files to publish, it uses .gitignore. I'll make a PR now with the folder we need.

[blakeembrey]

Fixed with #37.

[epoberezkin]

thank you, I published 1.1.1

[epoberezkin]

@blakeembrey should https://github.com/mulesoft-labs/osprey-method-handler/blob/master/package.json#L41 be changed to ^1.1.1? Or what is it better to do with that broken 1.1.0?

[blakeembrey]

@epoberezkin You can unpublish 1.1.0. I'll update that module once I use JSON pointers most likely.

[blakeembrey]

Thanks 👍

[epoberezkin]

:)