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.

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

WIP - update dev deps (with additional TODO list) #382

Closed
wants to merge 7 commits into from

Conversation

brodycj
Copy link
Contributor

@brodycj brodycj commented Jul 29, 2018

Platforms affected

iOS

What does this PR do?

Update devDependencies in master branch:

  • remove nodeunit
  • update jasmine, rewire, and tmp

to be moved into a separate PR:

(includes fix to simplify AppVeyor CI install, which was already integrated to master branch and will drop out of this PR next time I do a rebase)

This can be considered a continuation of the devDep cleanup merged from GH-368.

TO BE MOVED OUT OF THIS PR:

devDep TODO

  • While we are updating devDependencies I think we should move them below dependencies. More consistent with cordova-ios, I think more consistent with many other npm projects

Extra TODO items outside this PR:

What testing has been done on this change?

  • npm run unit-tests succeeds on mac "desktop"
  • check that npm test items succeed on Travis CI (skipping deprecated Node.js 4 version due to eslint update)
  • check that eslint & npm run unit-tests items succeed on AppVeyor CI (now passes on all covered Node.js versions including deprecated Node.js version 4 if fix proposed in Simplify installation on AppVeyor CI #383 is included) (skipping deprecated Node.js 4 version due to eslint update)

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

@codecov-io
Copy link

codecov-io commented Jul 29, 2018

Codecov Report

Merging #382 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #382   +/-   ##
=======================================
  Coverage   63.86%   63.86%           
=======================================
  Files          14       14           
  Lines        1702     1702           
  Branches      286      286           
=======================================
  Hits         1087     1087           
  Misses        615      615

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 bff9bca...7855e14. Read the comment docs.

@erisu
Copy link
Member

erisu commented Jul 31, 2018

@brodybits I just noticed that this PR and some of the PRs I submitted over a month ago are somewhat inline. I am not sure if you had taken a look at all of them.

#375 Remove Node 4 Support
Which I noticed in the PR description

#377 Increase Code Code coverage
This PR also includes dev dependencies updates but appears that your PR, contains newer versions. Since my PR is over a month old, I do suspect the dependencies to have updated since then.

  • eslint-config-semistandard
  • eslint-config-standard
  • eslint-plugin-node
  • jasmine
  • nyc (ADDED)
  • require
  • istanbul (DROPPED)

Since #377 also contains increased coverage, #376 will be needed because of a minor bug I found and happen to have written the test in PR #377.

@brodycj
Copy link
Contributor Author

brodycj commented Jul 31, 2018

Thanks @erisu. I did already reference #375 as GH-375, did not realize that #377 already included nyc.

I think you can see that I already reviewed and merged #376. I would kinda like to include the tests next, also hold off on dropping deprecated Node.js 4 until we take care of a couple items:

  • apply fixes to 4.5.x in case we decide to make another patch release there
  • get agreement to bump to next major release (5.0.0-dev)

I will very likely push a few changes to this WIP PR to check CI while checking GH-377 (#377)

@erisu erisu mentioned this pull request Aug 1, 2018
1 task
@brodycj brodycj changed the title WIP - update dev deps WIP WIP - update dev deps with additional TODO list Aug 1, 2018
@brodycj
Copy link
Contributor Author

brodycj commented Aug 1, 2018

This PR includes extra items that I will be moving out:

  • eslint-* updates
  • additional TODO list

@brodycj brodycj changed the title WIP - update dev deps with additional TODO list WIP - update dev deps (with additional TODO list) Aug 1, 2018
@brodycj brodycj requested a review from erisu August 2, 2018 03:50
@shazron
Copy link
Member

shazron commented Jan 9, 2019

Is this still WIP? There are conflicts that need to be resolved.

@brodycj
Copy link
Contributor Author

brodycj commented Jan 10, 2019

I will probably need 1-2 weeks to go through this. I wouldn't mind if someone else gets a chance to apply the changes on master, ideally one at a time.

@erisu
Copy link
Member

erisu commented Jan 18, 2019

Closing in favor #496 which is now merged.

@erisu erisu closed this Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants