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 project to ES6 standards #12

Merged
merged 1 commit into from
Jun 3, 2023
Merged

update project to ES6 standards #12

merged 1 commit into from
Jun 3, 2023

Conversation

HappyZombies
Copy link
Member

@HappyZombies HappyZombies commented May 30, 2023

Closes #4

TBH, lot of questionable stuff but I made changed a few simple things where it made sense. But as a whole I have a lot of questions on what this package is actually...doing. Would like to create a future item or discussion on how we can make this middleware even better.

But for the time being, yes see the code changes. Uses ES6 stuff; pointing arrow functions, class, await/async and more.

Let me know what y'all think 😃

Oh yeah and all unit tests pass and bluebird is removed (though technically it's still present since node-oauth/node-oauth2-server is a dependency of this...(which has bluebird))

@HappyZombies HappyZombies added the enhancement New feature or request label May 30, 2023
Copy link
Member

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your critic. I think the confusion comes due to missing documentation on how specific internals work or why they were chosen. I am looking to create another PR that deals with documentaiton. This is in itself good for me right now.

@jankapunkt
Copy link
Member

Should we publish as new major or just as a minor? Might be breaking for some using deprecated node engines.

@HappyZombies
Copy link
Member Author

@jankapunkt i think minor tbh but I could understand an argument for major.

This was previously set to version "0.11" which I think is one from 2013? I think tbh...time to upgrade buddy lol besides the "engine" property in NPM is a suggestion and will throw a warning to the user

https://docs.npmjs.com/cli/v9/configuring-npm/package-json#engines

@HappyZombies HappyZombies merged commit 3137483 into master Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update code to ES6
3 participants