Skip to content

Conversation

@Phillip9587
Copy link
Member

This PR refactors the code by moving the getCharset and typeChecker functions, which were previously duplicated across different parts of the codebase, into a shared lib/utils.js file.

Key Benefits:

  • Code reuse: Both functions are now centralized, reducing duplication.
  • Size optimization: By eliminating repeated code, this change helps shave off some bytes.
  • Maintainability: Future updates to these functions can be made in one place, simplifying maintenance.

No functionality has been altered—this is strictly a refactor for improved efficiency and modularity.

@Phillip9587
Copy link
Member Author

I'm unsure if internal refactorings need a history entry? What is your policy on this @UlisesGascon?

Copy link
Member

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

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

LGTM!

@UlisesGascon
Copy link
Member

What is your policy on this @UlisesGascon?

I think that making a reference is fine, so far if the change does not affect dependencies or API... it might be ignored as well. IMO, The aim for the HISTORY.md is to provide an easy way for the users to understand what has changed on how that might affect them. So in this case I am very neutral 👍

BTW... there is a discussion on going about the HISTORY.md, feel free to participate and provide your feedback expressjs/discussions#293

Also as you are doing a lot of contributions you might want to join us in Slack #express channel in the OpenJS foundation slack (invitation)

@Phillip9587
Copy link
Member Author

I think that making a reference is fine, so far if the change does not affect dependencies or API... it might be ignored as well. IMO, The aim for the HISTORY.md is to provide an easy way for the users to understand what has changed on how that might affect them. So in this case I am very neutral 👍

Thanks for the clarification.

Also as you are doing a lot of contributions you might want to join us in Slack #express channel in the OpenJS foundation slack (invitation)

Thanks for the invite. I'm happy to help.

@wesleytodd
Copy link
Member

Same question as in #551. We should align on if this or @ctcpip's revival of #544 is better.

@Phillip9587
Copy link
Member Author

Closed by #551

@Phillip9587 Phillip9587 deleted the utils branch March 26, 2025 21:52
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