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

feat: prepare jsr publish #989

Merged
merged 8 commits into from
Dec 15, 2024
Merged

Conversation

easymikey
Copy link
Contributor

@easymikey easymikey commented Dec 14, 2024

Fixes #980

Description

To support JSR:

  • create build jsr script
  • add tests
  • add return types
  • add comment for fix publish in future
Screenshot 2024-12-15 at 17 36 36
  • Tests pass
  • Appropriate changes to README are included in PR

Copy link

google-cla bot commented Dec 14, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@antongolub
Copy link
Collaborator

Hey, @easymikey,

  1. Let's keep the core.ts formatting
  2. CLA is required step

@easymikey easymikey force-pushed the prepare-jsr-publish branch 2 times, most recently from 4503181 to 0ebd232 Compare December 15, 2024 14:36
src/core.ts Outdated
code: () => null,
signal: () => null,
message: () => ProcessOutput.getErrorMessage(error, self._from)
self._resolved = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep the formatting for valid git blame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

Choose a reason for hiding this comment

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

core.ts
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I rebase another commit, everything is ok now

@easymikey easymikey changed the title Draft: prepare jsr publish Feat: prepare jsr publish Dec 15, 2024
@easymikey easymikey force-pushed the prepare-jsr-publish branch 2 times, most recently from 6ad4f65 to aa021be Compare December 15, 2024 15:40
@easymikey
Copy link
Contributor Author

@antongolub what do you think about adding test:coverege job to pull request action?

@easymikey easymikey requested a review from antongolub December 15, 2024 15:43
@antongolub
Copy link
Collaborator

@antongolub what do you think about adding test:coverege job to pull request action?

We've already done.

npm run test:coverage

@antongolub antongolub added the ossln24 OSS Library Night 2024 label Dec 15, 2024
@easymikey
Copy link
Contributor Author

@antongolub what do you think about adding test:coverege job to pull request action?

We've already done.

npm run test:coverage

Tell me, why are they launched only by the maintainer?

@easymikey easymikey changed the title Feat: prepare jsr publish feat: prepare jsr publish Dec 15, 2024
Copy link
Collaborator

@antongolub antongolub left a comment

Choose a reason for hiding this comment

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

Almost done. Just a few suggestions

--ext=<.mjs> default extension
--install, -i install dependencies
--version, -v print current zx version
--help, -h print help
--repl start repl
--experimental enables experimental features (deprecated)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep this extra line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d1


import fs from 'node:fs'
import path from 'node:path'
const cwd = process.cwd()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use root-based resolve, like here

const root = path.resolve(__dirname, '..')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d1

)

fs.writeFileSync(
path.resolve(cwd, 'jsr.json'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

cwd > root

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d1

@@ -70,6 +70,7 @@
"pretest": "npm run build",
"test": "npm run test:size && npm run fmt:check && npm run test:unit && npm run test:types && npm run test:license",
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. "test": "... && npm run test:jsr"
  2. add jsr.json to .gitignore

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahhh... its in temp. Ok, nvm

Copy link
Collaborator

@antongolub antongolub left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks for the contribution!

@antongolub antongolub merged commit 61c0ed1 into google:main Dec 15, 2024
21 checks passed
@easymikey
Copy link
Contributor Author

easymikey commented Dec 15, 2024

@antongolub Thanks for opportunity and your patience :)

@antongolub antongolub mentioned this pull request Dec 15, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ossln24 OSS Library Night 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: prepare for jsr.io publishing
2 participants