Skip to content

Removed Bluebird and Promisify-Any#107

Closed
jwerre wants to merge 12 commits into
developmentfrom
promisify
Closed

Removed Bluebird and Promisify-Any#107
jwerre wants to merge 12 commits into
developmentfrom
promisify

Conversation

@jwerre

@jwerre jwerre commented Dec 13, 2021

Copy link
Copy Markdown
Contributor

Summary

Removed support for callbacks and removed bluebird and promisify-any dependencies. I'm not sure how you want to merge this since this is a major version update (v5).

Linked issue(s)

Involved parts of the project

Added tests?

No tests added but many are refactored and changed.

OAuth2 standard

Reproduction

@jwerre

jwerre commented Dec 13, 2021

Copy link
Copy Markdown
Contributor Author

@jankapunkt CodeQL is picking up a hardcoded password in the test suite. Is there a way to ignore tests? What is the best solution for this?

@jorenvandeweyer

Copy link
Copy Markdown
Member

You can mark it as "used in test". Or make a variable for the authorization header.

@jankapunkt

Copy link
Copy Markdown
Member

I always use Math.random().to string(16) to generate a simple random hex password if the password itself is not to be tested.

@jankapunkt

Copy link
Copy Markdown
Member

@jwerre this is a great step towards modern JS but I'd like to delay it until we merged the open PRs that fix code and/or compliance issues otherwise we will have to resolve lots of merge conflicts what do you think?

@jwerre

jwerre commented Dec 14, 2021

Copy link
Copy Markdown
Contributor Author

@jankapunkt Yes, I was watching all the PRs coming in and thinking the same thing. No rush on this.

@Uzlopak

Uzlopak commented Dec 18, 2021

Copy link
Copy Markdown
Collaborator

If we merge #109 then this whole branch will be unmergeable.

@jankapunkt

Copy link
Copy Markdown
Member

@Uzlopak partially yes but it already needs refactoring/merging work so I think we should just set a cut after the recent 5 or 6 open PRs and exclusively focus on this one all together then it will be fixed fast and we can continue

@jankapunkt jankapunkt added code quality 🧽 Relating to code quality backwards breaking ✂️ This change will not work with the current version of the module. dependencies 🔌 Pull requests that update a dependency file labels Jan 7, 2022
@jankapunkt jankapunkt added this to the v5 milestone Mar 29, 2022
@esatterwhite

Copy link
Copy Markdown

@jankapunkt Is this still valid? can it be merged?

@jankapunkt

Copy link
Copy Markdown
Member

@esatterwhite this is still to be done, however there are a few things blocking:

I'm currently allocating more time but support from the community is also highly appreciated. Please watch the progress in #70, once merged we will definitely continue here.

@jankapunkt

Copy link
Copy Markdown
Member

Hey @jwerre I struggled hard to merge this and by the time I realized that the merge lost some code from development, the two branches are too far away from each other, also, because #109 changed many ES5 classes to ES6 classes and the diff was unbearable to comprehend...

I used your impl. as blueprint for creating #190 which I started from scratch, based on the latest development state.

I will close this as superseded by #190 please don't feel bad about it, the merge was too risky to loose recent fixes.

@dhensby dhensby deleted the promisify branch June 16, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards breaking ✂️ This change will not work with the current version of the module. code quality 🧽 Relating to code quality dependencies 🔌 Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants