johno / gulp-remarkable

A gulp wrapper for the remarkable markdown parser (CommonMark spec).
MIT License
10 stars 3 forks source link

resolves #1 - almost rewrite, lol #6

Closed tunnckoCore closed 9 years ago

tunnckoCore commented 9 years ago
johno commented 9 years ago

:heart_eyes_cat:

johno commented 9 years ago

This is awesome, @tunnckoCore . Thanks for your hard work!

johno commented 9 years ago

gulpplugin added to keywords.

johno commented 9 years ago

Closes #1.

tunnckoCore commented 9 years ago

haha, no prob :sweat_smile:

johno commented 9 years ago

Published to npm as version 1.0.0.

I've added a your twitter handle as a plug in the readme, too. Let me know if you'd like something different linked to. Thanks again!

tunnckoCore commented 9 years ago

@johnotander Thanks ;] What's you think for this https://github.com/tunnckoCore/gulp-remarkable? More style-ish, normalizing, plus automation, plus separation :) Simple npm test runs install, jshint, and tests.

Let me know if you want this PR, otherwise, no prob its just my style and "security", lol.

Btw, ready for merge -> pull -> npm publish, haha

johno commented 9 years ago

I love it.

The separation and jshinting is awesome. Though I wonder if the Makefile should be replaced with a gulpfile since it's a gulp plugin : ) ?

tunnckoCore commented 9 years ago

Hm, I forgot that fact, LOL :laughing: I will handle and that, no prob. But devDeps will count.

johno commented 9 years ago

Just a thought. If there's a good reason for a Makefile over a gulpfile in this scenario that I'm overlooking, please let me know.

IMO I don't think introducing a gulp devDependency is too worrisome for a gulp plugin.

Either way, thanks for the contributions. Greatly appreciated :beers: !

tunnckoCore commented 9 years ago

IMO I don't think introducing a gulp devDependency is too worrisome for a gulp plugin.

Agree.

Reason is "different people, different habits" - something for everyone. Also npm run and makefile is better than gulp, because stops if something isn't okey - i.e jshint-ish warrning/error, package.json parse error and so on. Actually I don't use gulp anywhere.

I don't know, maybe my implementation now for this gulpfile (v1.0.1 branch) is wrong? I mean, when jshint find "error/warning" (that is not actually real error) it continue to tests, try it. Set unused variable in some file and run gulp or gulp test and then try with npm test and makefile test.

johno commented 9 years ago

When I run the gulp command with your gulpfile, I get an error logged like I'd expect:

/Users/johno/code/frnt/gulp-remarkable/test/test.js
  line 17  col 5  'makeJsHintPissed' is defined but never used.

  ⚠  1 warning

But I guess your concern is that it doesn't terminate all processes when it arrives at an error? You can tell jshint to fail with:

gulp.task('lint', function () {
  gulp.src(['lib/*.js', 'test/*.js', 'index.js'])
    .pipe(jshint('.jshintrc'))
    .pipe(jshint.reporter('jshint-stylish'))
    .pipe(jshint.reporter('fail'));
});

However, it will still finish any other default tasks that are called.

johno commented 9 years ago

It seems most of the bases are covered since npm test and gulp lint will error out and the default gulp task will log an error.

I'd almost say that everything can be wrapped in the package.json with the npm commands. That'd be the lowest common denominator:

johno commented 9 years ago

In fact, I say we can probably just roll with the npm scripts don't you think? No need for a gulpfile or Makefile.

tunnckoCore commented 9 years ago

Yea we can, but why not three variants, lol

How I think: I always provide and Makefile (for unix world) and Npm equivalents (in package.json for cross platform usage). Because I see in most places something like

"scripts": {
  "test": "make test"
}

wtf? Why the heck? :D Npm scripts is for node.js world, plain javascript running for the sake of cross-platfomability and Makefile for unix world - the good old SoC principal.

Gulp is cool, but in meantime no so.

johno commented 9 years ago

Haha, no worries. Let's roll with the Makefile then.

tunnckoCore commented 9 years ago

And npm scripts - its by default ;d

To send PR for v1.0.1 branch?

johno commented 9 years ago

Yep. A nice built in.

Yeah!