Removed numeric separator#491
Removed numeric separator#491mcollina merged 2 commits intonodejs:mainnodejs/readable-stream:mainfrom
Conversation
|
Could you make this change inside the |
|
Ah I see. I've added it to the "replacements" file. Though, I'm unsure if I ran the build script properly as many files have minor style changes. Is there an instruction guide anywhere for running the build script? |
|
@ShogunPanda PTAL |
|
Also looks good to me in regard of changes in the @jaxoncreed can you please do the following?
After that you should only get the relevant change and then you can force push the branch. |
|
Thanks for the tip. I reset and ran the command |
|
It's mostly whitespace changes, I would guess there was some fix in babel or prettier that changes the output here. |
|
Is this okay to merge then? Is there anything else I should do? |
|
Hi all, just want to bump this. Can it be merged in? |
|
Can you please rebase and solve merge conflicts? |
|
Thanks @ShogunPanda I've rebased and solved merge conflicts |
|
@mcollina This LGTM. I think you can now have the CI run and then merge. |
|
Hi all. Just bumping this again to see if it can be merged in. |
|
I'm a bit swamped atm. Releasing readable-stream is always a potentially dangerous operation (given the amount of downloads) that I want to do with great care. It's on the list. |
|
Hey, sorry to pester again. Just want to see if there's a timeline for merging this in. |
|
breaks tar-stream@^3 for me |
|
@moki how? did you have a repro? |
|
Thank you |
This is a simple pull request that removes the numeric separator from validators.js.
While the numeric separator is nicer to look at, it causes problems when this library is used in React-Native's default configuration. facebook/metro#645. It is possible to change the default configuration manually, but that's overhead for a developer. To make it easier to install developer tooling that uses readable-stream it would be great to remove the numeric separators.