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

Assorted fixes #378

Closed
wants to merge 8 commits into from
Closed

Assorted fixes #378

wants to merge 8 commits into from

Conversation

XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Jan 4, 2021

@jkphl: unsure if the package is still maintained but since we are still using it, I thought I'd make a PR. I could split some of the changes if needed.

There are failing tests even on master on Node.js >=12. I think the test PNGs need to be updated and also:

  • svg2png; the current version breaks on Node.js 15 I updated it, but needs another pair of eyes
  • node-sass; the current version does not work on Node.js 15
  • mocha; still works though
  • istanbul should be replaced with nyc because it's deprecated replace it with c8
  • image-diff is deprecated and should be replaced too (could be one of the reasons image diffs fail on Node.js >= 12?)
  • pn can be removed and use Node.js's util.promisify
  • updated svgo to v2.x, needs another pair of eyes
  • JSHint needs to be replaced at some point too

The rest of the deps shouldn't matter but my guess is Node.js < 10 support will need to be dropped. Edit, dropped it since more and more deps don't work there.

This should fix any lgtm.com issues. I also added a CodeQL action which is the successor to CodeQL, but ideally you could use both.

@XhmikosR
Copy link
Member Author

@jkphl friendly ping :)

I can split some of the patches; I started this for a few fixes but I've landed quite a few patches, so it might be easier for you if I split the branch somehow. That being said, I tried to keep the patches as clean as possible.

Please also check the PR description.

@XhmikosR XhmikosR marked this pull request as draft February 22, 2021 16:51
@coveralls

This comment has been minimized.

@XhmikosR
Copy link
Member Author

Closing this since I've split most patches. The only remaining patch is the svgo update which I'll tackle in the next days in a separate PR.

@XhmikosR XhmikosR closed this Feb 24, 2021
@XhmikosR XhmikosR deleted the dev branch February 24, 2021 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants