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

make file structure closer to Node.js' #6

Merged
merged 4 commits into from
Apr 20, 2022

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Apr 20, 2022

Having the file structure as close as nodejs/node one will help keeping
the code up-to-date. We can also use package imports for the require
calls to be as close as the upstream ones as possible.

Having the file structure as close as nodejs/node one will help keeping
the code up-to-date. We can also use package imports for the require
calls to be as close as the upstream ones as possible.
@@ -1,5 +1,5 @@
#!/usr/bin/env node
// https://github.com/cjihrig/node/blob/484d35402de36d8d5756b244c8e5fbb8aa4c6afd/lib/internal/main/test_runner.js
// https://github.com/nodejs/node/blob/HEAD/lib/internal/main/test_runner.js
Copy link
Member

Choose a reason for hiding this comment

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

+1 for changing the repo, I liked keeping the sha in there so you could tell exactly which file this was ported from. What do you think?

bin/test_runner.js Show resolved Hide resolved
const expectedLength =
(StringPrototypeMatch(msg, /%[dfijoOs]/g) || []).length
const expectedLength = (StringPrototypeMatch(msg, /%[dfijoOs]/g) || [])
.length
Copy link
Member

Choose a reason for hiding this comment

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

about the style changes, shall we move away from standard to match node core closer (with semicolons and all)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh, that's my VSCode config that's messing with me, trying to run Prettier on standard files

Copy link
Member

Choose a reason for hiding this comment

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

I was using prettier-standard before but realized it was unmaintained

@@ -0,0 +1,221 @@
// https://github.com/nodejs/node/blob/HEAD/lib/internal/test_runner/tap_stream.js
Copy link
Member

Choose a reason for hiding this comment

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

some of these files exist twice now

@aduh95 aduh95 requested a review from juliangruber April 20, 2022 18:28
@juliangruber juliangruber merged commit a3a9be7 into nodejs:main Apr 20, 2022
@aduh95 aduh95 deleted the file-structure-update branch April 20, 2022 18:42
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.

2 participants