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

Add default value to the isAbsolute function of utils #5206

Conversation

raksbisht
Copy link
Contributor

No description provided.

@dougwilson
Copy link
Contributor

Hello, and thank you for your pull request. If possible, please add a test to demonstrate what this is fixing. Typically these are internal functions only used by Express.js, though there are published modules on npm who are reaching in and using these. If possible, we don't want to touch these unless it is a critical bug fix, and this particular utility is already removed in 5.0, so this change would only be for the 4.x line which we are trying not to change unless critical. Can you elaborate on the purpose of the change, and if it is critical to get in the 4.x line?

@raksbisht
Copy link
Contributor Author

Hello,

Thank you for your feedback and considerations regarding the pull request. I understand that the proposed change may not be critical and that modifications to internal functions should be avoided unless necessary. I agree that it is important to prioritize critical bug fixes and avoid unnecessary changes, especially considering that the utility in question will be removed in the 5.0 version of Express.js.

Upon further evaluation, I realize that the issue addressed by the proposed change does not fall into the category of critical bug fixes. Therefore, I agree that it would be appropriate to postpone this change in the 4.x line unless it becomes critical.

I apologize for any inconvenience caused, and I appreciate your guidance and insights regarding the best practices for modifying internal functions in Express.js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants