Add hjson support

[ghost]

Adding HJSON support, does not support json override Started in https://github.com/haraka/Haraka/issues/2124

P.S.: All test work but someone should really check what I wrote, but I assume you do it anyways

[codecov[bot]]

Codecov Report

Merging #30 into master will increase coverage by 0.54%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   80.73%   81.27%   +0.54%     
==========================================
  Files           7        8       +1     
  Lines         410      422      +12     
  Branches      118      123       +5     
==========================================
+ Hits          331      343      +12     
  Misses         79       79

Impacted Files	Coverage Δ
readers/hjson.js	100% <100%> (ø)
config.js	89.74% <100%> (+0.7%)	:arrow_up:
configfile.js	70.24% <100%> (+0.14%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ce7243a...a584727. Read the comment docs.

[ghost]

The Codecov is now messed up as I accidentally pushed to the same branch. The accident is removed but the Codecov stayed. Sorry...

[msimerson]

Whoohoo, looks great except that you've also added a package-lock.json, which is likely something that npm just did "for you." Maybe we should add that to .npmignore for now?

[ghost]

Yes, definitely it should be in .gitignore or something.

[msimerson]

Do feel free to add it. I think it's NPM 5+ that drops that file by default, so we'll likely be adding that to .gitignore in all the repos in haraka/*.

[ghost]

Ok, should I include it in this PR or should I create a new one?

[msimerson]

Either way is fine.

[msimerson]

fixes haraka/Haraka#2124

[msimerson]

thanks @PSSGCSim

[baudehlo]

https://stackoverflow.com/questions/44206782/do-i-commit-the-package-lock-json-file-created-by-npm-5

This link seems to suggest we should keep the package-lock file.

On Sep 28, 2017, at 6:04 PM, Matt Simerson notifications@github.com wrote:

thanks @PSSGCSim

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.

[ghost]

If you read it, I do not think this file is all that needed. Because npm creates it itself while running npm install and it only stores integrities and such regarding the installation (the sub-modules). If we would need to do so I think package.json should be sufficient. I think this would be important only if you would like to host configuration of an instance on git. Like for updating and fetching exactly same modules at each deployment. Correct me if I am wrong.

[baudehlo]

I’m no fan of npm 5. It caused me tons of headache and lost time at work. I just wanted to show what the general consensus was.

On Sep 28, 2017, at 8:50 PM, Jan Schwarzrock notifications@github.com wrote:

If you read it, I do not think this file is all that needed. Because npm creates it itself while running npm install and it only stores integrities and such regarding the installation (the sub-modules). If we would need to do so I think package.json should be sufficient. I think this would be important only if you would like to host configuration of an instance on git. Like for updating and fetching exactly same modules at each deployment. Correct me if I am wrong.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.

[ghost]

I have linked the npm's docs onto this issue in the .gitignore. https://github.com/haraka/haraka-config/commit/e82a40252b40fd2e70e08514f51d313560cb5a6b#diff-a084b794bc0759e7a6b77810e01874f2 I am not all that experienced with Node.js/npm but from what I got it is not needed. Again, please correct me if I am wrong.

[baudehlo]

Totally not needed. It’s a question of does it benefit or not. I don’t have an opinion either way.

On Sep 28, 2017, at 9:02 PM, Jan Schwarzrock notifications@github.com wrote:

I have linked the npm's docs onto this issue in the .gitignore. e82a402#diff-a084b794bc0759e7a6b77810e01874f2 I am not all that experienced with Node.js/npm but from what I got it is not needed. Again, please correct me if I am wrong.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.

[msimerson]

While acknowledging that the consensus appears to be: check it in, my opinion is that if we do decide to, we should wait until after we drop support for node 6 (and with it, npm 3). For now, I'm personally avoiding npm 5 for a while longer, thanks to show-stopper bugs. Shucks, NPM 5 is what finally convinced me to give yarn a try. For now, I'm sticking with npm 3 for a while longer, while npm 5 gets most of the pathological bugs ironed out.

[baudehlo]

I agree with you Matt. I've had the same problems. npm5 was a clusterfuck for us.

On Fri, Sep 29, 2017 at 1:50 PM, Matt Simerson notifications@github.com wrote:

While acknowledging that the consensus appears to be: check it in, my opinion is that if we do decide to, we should wait until after we drop support for node 6 (and with it, npm 3). For now, I'm personally avoiding npm 5 for a while longer, thanks to show-stopper bugs. Shucks, NPM 5 is what finally convinced me to give yarn a try. For now, I'm sticking with npm 3 for a while longer, while npm 5 gets most of the pathological bugs ironed out.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/haraka/haraka-config/pull/30#issuecomment-333193819, or mute the thread https://github.com/notifications/unsubscribe-auth/AAobY8kbuRUeoM6MhT463pJOa0AF9C4Vks5snS3TgaJpZM4PnH6B .