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

Include test utils in NPM package #211

Merged

Conversation

andreivladbrg
Copy link
Contributor

Changelog:

build: include test utils in package
build: install prb test as npm package
build: install forge as npm package
build: remove gitmodules
ci: install node js dependencies in build and test job
chore: update remappings accordingly

As mentioned in my comment here, importing files from the ~test/ path is not possible in the NPM package because this folder doesn't exist.

Inevitably, to make this fully compatible with the idea presented in the discussion linked above, I had to install prb-test as an NPM package.

build: install prb test as npm package
build: install forge std as npm package
build: remove gitmodules
ci: install node js dependencies in build and test job
chore: update remappings accordingly
@andreivladbrg andreivladbrg force-pushed the build/include-test-utils-in-package branch from 977737d to 233b630 Compare November 16, 2023 16:05
@andreivladbrg
Copy link
Contributor Author

Should I bump the package version to 4.0.2 and add the changes into CHANGELOG?

package.json Outdated Show resolved Hide resolved
Copy link
Owner

@PaulRBerg PaulRBerg left a comment

Choose a reason for hiding this comment

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

thanks, see feedback in the comments

Should I bump the package version to 4.0.2 and add the changes into CHANGELOG?

I will do that separately

@PaulRBerg
Copy link
Owner

Thanks for the PR, Andrei.

importing files from the ~test/ path is not possible in the NPM package because this folder doesn't exist.

Obviously, I haven't published the new package. The folder will exist when I publish @prb/math@4.0.2.

To test what the directory structure will be in an npm package, use this command:

$ npm pack --dry-run

I had to install prb-test as an NPM package.

That's fine.

Copy link
Owner

@PaulRBerg PaulRBerg left a comment

Choose a reason for hiding this comment

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

generally looks good to me now, except for the unnecessary use of @forge-std.

package.json Outdated
@@ -10,11 +10,14 @@
"url": "https://github.com/PaulRBerg/prb-math/issues"
},
"devDependencies": {
"@forge/std": "github:foundry-rs/forge-std#e8a047e3f40f13fa37af6fe14e6e06283d9a060e",
Copy link
Owner

@PaulRBerg PaulRBerg Dec 4, 2023

Choose a reason for hiding this comment

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

There's no need to use a faux @forge-std organization. We can just type it as forge-std.

@PaulRBerg PaulRBerg merged commit a22a32f into PaulRBerg:main Dec 4, 2023
3 checks passed
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