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

Update dependencies #20

Merged
merged 1 commit into from
Jan 23, 2019
Merged

Update dependencies #20

merged 1 commit into from
Jan 23, 2019

Conversation

jakezatecky
Copy link
Collaborator

Greetings!

This PR updates the various dependencies to their latest versions and slightly modifies index.js. In particular, several of the depended packages are quite old and I noticed quite a few npm audit warnings related to some of them (related to #19) on a project that depended on this.

This effort is somewhat of a continuation of the work of #15. It promisifies options.delFn to work like the newer del (without an additional dependency), it updates the tested node versions, it fixes some inconsistencies in formatting (such as some places having spaces AND tabs), and it makes use of const/let and arrow functions.

The last few items are ultimately unnecessary for this PR but I included them partially because of my lovely editor throwing squiggles everywhere. If you want them removed, I can easily do that and squash the changes to a single commit.

Alternatively, if this remains sitting for a while, I may go ahead and maintain a separate fork (with full credits and a link to this project, naturally) because my compulsiveness wants me to remove those audit errors from the other project.

Promisify options.delFn, update tested versions, use consistent formatting, and make use of const/let.
@callumacrae
Copy link
Owner

Hey, thanks for the PR!

I've invited you as a collaborator to this repository, because I suck at maintaining it and it's got a bit out of date. I haven't tested your changes (I don't actually use rev-del anymore) but they all look good to me.

Feel free to do whatever and let me know when to publish to npm :)

@jakezatecky
Copy link
Collaborator Author

Thanks for the invitation!

I figured that you likely did not use this library anymore, but it's good to see your responsiveness. I still use it on one of the projects I maintain.

The tests all passed on my local machine, but I will get about testing the package on the older project to see if things still work a bit later. I'll let you know!

@jakezatecky
Copy link
Collaborator Author

Hey there. I did a quick test to ensure that this build still functions properly with a live project, though naturally it was not fully exhaustive. Everything appears to be working fine.

It should be noted that some of the changes here does mean that the library will no longer work on Node v4. That version fell out of official support last year on 30 April 2018, so I do not know how much of a concern that is. It would probably be considered a breaking change as far as this library goes, though.

I'll leave the decision to you whether or not to merge the changes based on that caveat If that's all fine, I think we would be good for a new release!

@callumacrae callumacrae merged commit a53017b into callumacrae:master Jan 23, 2019
@callumacrae
Copy link
Owner

Merged and published as 2.0.0 :)

@perenstrom
Copy link

Is it possible to link this PR in the release notes? :)

@callumacrae
Copy link
Owner

Done! https://github.com/callumacrae/rev-del/releases/tag/v2.0.0

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.

3 participants