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

Finish move to nodejs org #9

Merged
merged 18 commits into from
Jun 2, 2022
Merged

Finish move to nodejs org #9

merged 18 commits into from
Jun 2, 2022

Conversation

juliangruber
Copy link
Member

@juliangruber juliangruber commented May 19, 2022

  • Update license
  • Bin name

@juliangruber juliangruber requested a review from aduh95 May 19, 2022 09:35
package.json Outdated Show resolved Hide resolved
@aduh95
Copy link
Contributor

aduh95 commented May 20, 2022

We should also fix the docs and the GHA workflows.

@juliangruber
Copy link
Member Author

@aduh95 I'm not allowed to rename the repo, are you?

@juliangruber
Copy link
Member Author

Also, how shall we update the LICENSE file?

@juliangruber juliangruber changed the title Pkg: Fix .repository Finish migration to nodejs org May 20, 2022
@juliangruber juliangruber changed the title Finish migration to nodejs org Finish move to nodejs org May 20, 2022
@aduh95
Copy link
Contributor

aduh95 commented May 20, 2022

@aduh95 I'm not allowed to rename the repo, are you?

I don’t think we should rename the repo, test is too broad of a name for a repo imo.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
possible to skip all top level tests except for a selected subset by passing
the `only` option to the tests that should be run. When a test with the `only`
option set is run, all subtests are also run. The test context's `runOnly()`
method can be used to implement the same behavior at the subtest level.

```js
// Assume node-core-test is run with the --test-only command-line option.
// Assume test is run with the --test-only command-line option.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need ticks to remove the ambiguity around the package name.

Suggested change
// Assume test is run with the --test-only command-line option.
// Assume `test` is run with the `--test-only` command-line option.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, test is a bash builtin too, so it's also not a good cli name 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure what to do about this. do you have an idea?

Copy link
Contributor

@aduh95 aduh95 May 22, 2022

Choose a reason for hiding this comment

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

We could keep node-core-test, use npx test (and yarn test on Yarn projects), or possibly both.

package.json Outdated Show resolved Hide resolved
@aduh95
Copy link
Contributor

aduh95 commented May 20, 2022

Also, how shall we update the LICENSE file?

We probably should.

package.json Outdated Show resolved Hide resolved
juliangruber and others added 5 commits May 22, 2022 15:43
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
README.md Outdated Show resolved Hide resolved
Comment on lines 7 to +9
const { bin } = require('../../package.json')

process.execPath = path.resolve(__dirname, '..', '..', bin['node-core-test'])
process.execPath = path.resolve(__dirname, '..', '..', bin.test)
Copy link
Contributor

Choose a reason for hiding this comment

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

We could get rid of the magic string here (feel free to ignore, that's going to be useful only in the unlikely event of this pacakge switching name ever again).

Suggested change
const { bin } = require('../../package.json')
process.execPath = path.resolve(__dirname, '..', '..', bin['node-core-test'])
process.execPath = path.resolve(__dirname, '..', '..', bin.test)
const { bin, name } = require('../../package.json')
process.execPath = path.resolve(__dirname, '..', '..', bin[name])

juliangruber and others added 3 commits May 22, 2022 19:55
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
README.md Outdated Show resolved Hide resolved
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@juliangruber juliangruber merged commit 15644ec into main Jun 2, 2022
@juliangruber juliangruber deleted the juliangruber-patch-1 branch June 2, 2022 13:23
aduh95 added a commit that referenced this pull request Jun 2, 2022
Update documentation and binary names to reflect the change from
`node-core-test` to `test`.

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
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