refactor(lint): replace eslint with standard #2579
Conversation
one additional standard lint error after merging with upstream
avoid using the `err` var name if we have no use for it
|
+1 I just went through doing all this myself and this is actually a bit better. |
| @@ -32,11 +31,11 @@ OAuth.prototype.buildParams = function (_oauth, uri, method, query, form, qsLib) | |||
| oa.oauth_signature_method = 'HMAC-SHA1' | |||
| } | |||
|
|
|||
| var consumer_secret_or_private_key = oa.oauth_consumer_secret || oa.oauth_private_key | |||
There was a problem hiding this comment.
We shouldn't alter these variable names. They map to actual OAuth standard names so we should keep them.
We should stick /* eslint-disable camelcase */ and /* eslint-enable camelcase */ around the code to just disable this for this code block.
There was a problem hiding this comment.
this has been done ... not sure why github still shows this as unresolved.
remain consistent with OAuth standard, as per @mikeal recommendation
|
@mikeal updated. I opted for line-level lint disable, rather than wrapping the whole block, as other variables are declared within that should follow the |
There was a problem hiding this comment.
@ahmadnassri can you remove the .eslintrc file as well?
LGTM once that's gone, @mikeal do you still have comments?
|
@FredKSchott done. |
|
Looks like there are conflicts too, can you resolve? |
|
ah, didn't see on mobile (where I made that last commit) @FredKSchott managed to do it on mobile 📱 🎉 |
|
Nice! Big lint PRs have a tendency to stagnate quickly, will merge asap (the issue discussing this has been open for a while now without any opposition so this shouldn't be catching anyone by surprise) |
|
Thanks @ahmadnassri! |
PR Checklist:
npm testlocally and all tests are passing.PR Description
eslintwithstandard