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

deps: vendor emphasize #52826

Closed
wants to merge 1 commit into from
Closed

Conversation

avivkeller
Copy link
Member

Blocks: #52593

This PR is a proposal to vendor the emphasize library to be used with #52593's experimental REPL.

Emphasize's (un-minified) build size is roughly 5.5MB. When bundled, the bundle.js file is roughly 1.5MB.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions
  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels May 3, 2024
@benjamingr
Copy link
Member

I genuinely appreciate your effort but the number of open contributions and lack of communication about what you're working on are making it really difficult to collaborate with you effectively.

Look at how well it went when you worked on something in scope and with coordination like --test-skip-pattern and consider how much wasted effort (for the reviewers too) picking up old stuff (like the experimental repl) or making structure/architecture changes creates.

To be clear I think none of this is in bad faith and I think you're genuinely trying to help (and you're a decent coder). I urge you to reconsider how you're communicating with the rest of the people working on the project and the way you open PRs.

@avivkeller
Copy link
Member Author

avivkeller commented May 3, 2024

I genuinely appreciate your effort but the number of open contributions and lack of communication about what you're working on are making it really difficult to collaborate with you effectively.

Look at how well it went when you worked on something in scope and with coordination like --test-skip-pattern and consider how much wasted effort (for the reviewers too) picking up old stuff (like the experimental repl) or making structure/architecture changes creates.

To be clear I think none of this is in bad faith and I think you're genuinely trying to help (and you're a decent coder). I urge you to reconsider how you're communicating with the rest of the people working on the project and the way you open PRs.

Thanks for the insight! I'll make sure to consult with the community before taking on any tests, and I'll make sure I have a clear 'game-plan' for any activities. I appreciate the effort to deal with people like me who are 'bouncing off the walls'.

I'd like to keep this PR open for discussion, as I believe that a new REPL isn't a bad idea, but I'll refrain from these types of changes without first finding a need and consulting with maintainers.

@GeoffreyBooth
Copy link
Member

If I could selfishly hope to redirect your energy, there are some TODO lists already around with some enhancements that already have consensus that you could dive into:

While an improved REPL would be nice, I think there are other subsystems that are higher priorities for improvement. I focus on modules and customization/loaders, so of course I think that’s a high priority (better ESM and/or TypeScript support, anyone?) but there are others as listed in https://github.com/nodejs/node/blob/main/doc/contributing/technical-priorities.md. Many of these have open issues or tracking issues already; you can ask about the ones that interest you on Slack at #nodejs-core. https://slack-invite.openjsf.org/

@avivkeller
Copy link
Member Author

Dropping, I'll remove syntax highlighting from the experimental repl (for now)

@avivkeller avivkeller closed this May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants