Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor: eslint setup #105

Merged
merged 5 commits into from
Nov 15, 2019
Merged

refactor: eslint setup #105

merged 5 commits into from
Nov 15, 2019

Conversation

erisu
Copy link
Member

@erisu erisu commented Nov 8, 2019 •
edited
Loading

Motivation and Context

Code syntax cleanup

Closes #73

Description

  • replace dependencies with @cordova/eslint-config
  • update eslint config
  • eslint corrections

Testing

  • npm t
  • npm run eslint

Checklist

  • I've run the tests to see all new and existing tests pass

@erisu erisu added this to the 4.0.0 milestone Nov 8, 2019
@erisu erisu requested a review from raphinesse November 8, 2019 03:43
raphinesse
raphinesse reviewed Nov 8, 2019
View reviewed changes
package.json Outdated Show resolved Hide resolved
src/PluginInfo/PluginInfo.js Outdated Show resolved Hide resolved
src/PluginInfo/PluginInfo.js Outdated Show resolved Hide resolved
@erisu erisu requested a review from raphinesse November 8, 2019 14:51
@raphinesse
Copy link
Contributor

Should we wait for @cordova/eslint-config@2 before merging this?

@codecov-io
Copy link

codecov-io commented Nov 10, 2019 •
edited
Loading

Codecov Report

Merging #105 into master will not change coverage.
The diff coverage is 88.88%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #105   +/-   ##
=======================================
  Coverage   86.28%   86.28%           
=======================================
  Files          21       21           
  Lines        1560     1560           
=======================================
  Hits         1346     1346           
  Misses        214      214
Impacted Files Coverage Δ
src/ConfigParser/ConfigParser.js 80.24% <100%> (ø) ⬆️
src/ConfigChanges/ConfigChanges.js 94.56% <100%> (ø) ⬆️
src/PluginInfo/PluginInfo.js 81.59% <66.66%> (ø) ⬆️

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 31a58cd...7bdab86. Read the comment docs.

@erisu
Copy link
Member Author

erisu commented Nov 11, 2019

Yeah,

I think that will be best.

I actually have been waiting for the @cordova/eslint-config@2 to drop all the other dependencies. I will start preparing the @cordova/eslint-config@2 vote today and hopefully we can get it released in 2 days.

@erisu erisu force-pushed the v4-eslint branch 2 times, most recently from 6204699 to fe5036c Compare November 14, 2019 06:25
@erisu erisu changed the title chore: bump eslint dependencies w/ fix refactor: eslint setup Nov 14, 2019
@erisu
refactor: eslint setup
8f5d2db 
* replace dependencies with @cordova/eslint-config
* update eslint config
* eslint corrections
@erisu erisu merged commit 291158c into apache:master Nov 15, 2019
@erisu erisu deleted the v4-eslint branch November 15, 2019 02:09
@raphinesse raphinesse mentioned this pull request Nov 27, 2019
Open
40 tasks
@brodycj brodycj mentioned this pull request Dec 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timbru31 timbru31 timbru31 approved these changes

@raphinesse raphinesse raphinesse approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@erisu @raphinesse @codecov-io @timbru31