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: add publish command #182

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Donovan-Ye
Copy link

Trying to add the missing publish command as listed in README.md.

Copy link

changeset-bot bot commented Sep 17, 2024

⚠️ No Changeset found

Latest commit: 6106d4d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@iosh
Copy link
Collaborator

iosh commented Sep 17, 2024

Hi, nice PR, but I need more time to review it. I'll try to complete it by the end of the month.

@iosh
Copy link
Collaborator

iosh commented Sep 22, 2024

Hi @Donovan-Ye can you add some test for this feature?

@Donovan-Ye
Copy link
Author

Hi @Donovan-Ye can you add some test for this feature?

yeah, of course, I will try to add some tests.

@Donovan-Ye
Copy link
Author

Donovan-Ye commented Sep 24, 2024

Hi @iosh, I have added tests for the publish command. Please review them when you have time ~

@iosh
Copy link
Collaborator

iosh commented Oct 4, 2024

Hi @Donovan-Ye Sorry for keeping you waiting. This PR only implements the "publish" command, lacking support for other options such as -t, -a, -o, etc. I have looked at the implementation of pnpm and found a plugin @pnpm/plugin-commands-publishing. Perhaps we can directly base our "publish" command implementation on it. What do you think about this?

@Donovan-Ye
Copy link
Author

Hi @Donovan-Ye Sorry for keeping you waiting. This PR only implements the "publish" command, lacking support for other options such as -t, -a, -o, etc. I have looked at the implementation of pnpm and found a plugin @pnpm/plugin-commands-publishing. Perhaps we can directly base our "publish" command implementation on it. What do you think about this?

You are right. Let me have a look at the @pnpm/plugin-commands-publishing plugin first~ We can discuss it later.

@Donovan-Ye
Copy link
Author

I have a brief look at the plugin, I believe we can achieve our "publish" command base on it. However, many internal functions are dependent on pnpm. We will need to either develop these functionalities ourselves or seek an alternative solution. But for sure, we can follow their ideas.

@iosh
Copy link
Collaborator

iosh commented Oct 8, 2024

I have a brief look at the plugin, I believe we can achieve our "publish" command base on it. However, many internal functions are dependent on pnpm. We will need to either develop these functionalities ourselves or seek an alternative solution. But for sure, we can follow their ideas.

Waiting for your good news and let me know if you need any help

@iosh
Copy link
Collaborator

iosh commented Nov 24, 2024

Hi @Donovan-Ye , I have read the changeset source code, I think we can implement like this : https://github.com/changesets/changesets/blob/main/packages/cli/src/commands/publish/npm-utils.ts#L237

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