New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
style(code): implements eslint & prettier #31
Conversation
@@ -1,175 +1,1770 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should ignore this since this files is managed by npm, not by hand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure I did touch it via npm only, no prettier or eslint included. Will regenerate it to ensure this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, no changes did show up after deleting the file and reinstalling. Should be fine as it is :)
src/index.js
Outdated
const sizeOf = require('image-size') | ||
const argv = require('argv') | ||
const os = require('os') | ||
const childProcess = require('childProcess') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Package name is child_process
so this is going to brake,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😱 omg yes, sorry for this one 😅
BREAKING CHANGE: lib return values are now camelCased
9926c9f
to
dedb8af
Compare
src/index.js
Outdated
const sizeOf = require('image-size') | ||
const argv = require('argv') | ||
const os = require('os') | ||
const childProcess = require('child-process') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is still wrong ? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also eslint will fail on camelcase
I assume, for that we can disable check for that line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what u mean here. Camelcase is enforced for variable names, not for module names/string content 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it is not failing on ESLint as you said sorry :)
But child_process
module is with underscore instead of dash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omg. Sure! Note to myself: Test the code before pushing.
Usually I do that but since we plan do do more for the next version I actually did not test this commit. Sorry
@efegurkan fixed 😅 |
@axe312ger LGTM, feel free to merge it :) |
BREAKING CHANGE: lib return values are now camelCased
BREAKING CHANGE: lib return values are now camelCased
BREAKING CHANGE: lib return values are now camelCased
BREAKING CHANGE: lib return values are now camelCased
This is my first step to get this lib in 2018 shape. 99% of the changes are auto fixed from eslint or prettier. I'd keep this in the next branch and start releasing 1.0.0-alpha's as soon we got the plugin system running.
Next step might be upgrading all dependencies.