Thanks for this @SimonSchick! I'm a little hesitant about the targeting ES6 purely because it requires that all consumers need to be on > node 6.4.

Mind you, for webpack 4 that ship sailed back in February so it's probably fine. And node 10 is now the LTS install. Yeah it's probably fine.

@piotr-oles do you have any thoughts?

@johnnyreilly The package does specify a minimum node version.

https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/97c86d66f1b870898414e32e8fc777e7253d9104/package.json#L40

or is that considered none binding?

In any case you can release it as 0.5 when it doubt.

I think it's probably fine - I've already done this myself here: https://github.com/johnnyreilly/fork-ts-checker-notifier-webpack-plugin/blob/master/tsconfig.json

I'd like to hear what @piotr-oles thinks about this but I think this looks generally fine. Could you address my comments please?

Hi @SimonSchick,

Please could you address my review comments? Thanks

@johnnyreilly I don't see any review comments, are you sure you've submitted them?

How about now?

I'd consider all comments addressed.

@johnnyreilly done

Any reservations about the target: es2015 stuff @piotr-oles?

I'd also like to point out again that node v6 is not in the build matrix and I haven't tested my changes on node v6, I validated my changes again node.green however.
If you want to support v6 in the future I can ammend it to this PR, otherwise we might as well skip to the es2017 build target.

@johnnyreilly thoughts?

I was hoping @piotr-oles would comment but I guess he's busy. I think I'm fine with this though. I'd like to release it as 0.5 just in case there's any old node users out there.

Do you want to update the package.json and the CHANGELOG.md please?

I bumped up dev deps and added ts-loader@5 to the test matrix as well.

Also do you have any plans to drop webpack 2-3 since v5 is around the corner?

Also do you have any plans to drop webpack 2-3 since v5 is around the corner?

Not as long as supporting both remains straightforward (and so far it does).

Also, I'm not that comfortable with the dropping of support for node 6 without @piotr-oles commenting first I'm afraid.

All good, was just wondering if there were any plans :)

@piotr-oles I went ahead and added public/private eveywhere and enabled the corresponding lint rule.

While doing so I also noticed that the properties of NormalizedMessage were accessed sometimes directly and indirectly (via getter), I marked all properties as public readonly instead, might even give a speed boost :)

Also please note, adding node v6 to the build matrix doubled the job count, adding v10 will triple it, you might want to consider dropping wp 2/3 in master and maintain them in a separate branch.

Thank you @SimonSchick for your work. I'm aware that it will make builds a lot longer but it shouldn't be an issue in terms of development. You can still run your test locally on one node version and in 99% cases it will pass on all node versions :)

Let me just fix the build :)

So turns out es2015 !== es6 build target 😅

@piotr-oles you should look into porting tests to TS too, turns out tests don't work in node v6.

Sorry for the mess, I'm in the process of moving and haven't had a whole lot of time to work on this more than 5min in a block.

This is why "squash and merge" was invented 😉

@johnnyreilly builds now~~ 🎉

Great - I've put a few more comments around the package.json and changelog.md. if you could address these then I think we're good to go!

@johnnyreilly I don't see any new comments, did you forget to request the changes? Happens to me all the time 😛

Now?

I think I got everything now, on a intercontinental flight atm, might not respond immediately.

Happy landings! https://github.com/Realytics/fork-ts-checker-webpack-plugin/releases/tag/v0.5.0

Thanks for your help ❤️