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(dev-tools): add new modes to ocular-publish #461

Merged
merged 3 commits into from
Apr 24, 2024
Merged

Conversation

Pessimistress
Copy link
Collaborator

  • Upgrade lerna to the latest
  • New publish modes to support tag-locally, publish-on-CI scenario

cc @felixpalmer @donmccurdy

Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

Nice to see latest lerna. Docs could use a little TLC so less context is needed.

@@ -18,6 +18,9 @@ npx ocular-publish [mode] [npm-tag]

- `beta` - bump pre-release version and publish with beta flag.
- `prod` - bump patch version and publish.
- `version-beta` - bump pre-release version only.
Copy link
Collaborator

Choose a reason for hiding this comment

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

These docs looks quite terse, perhaps we can be a bit more verbose. Without reading the source I can't quite tell what these do.

Nit: The docs make it look like these two modules don't actually publish? If that is true, should we call them ocular-bump instead of ocular-publish then? or maybe ocular-publish bump-beta or ocular-publish bump beta ocular-publish bump-only beta

Choose a reason for hiding this comment

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

In npm, yarn, and lerna, the "version" verb means incrementing the release number, and the "publish" verb means pushing that change to a registry. A postversion script might also run publish if projects prefer to do it all in one step, which seems common.

I'd prefer if ocular could reuse these terms as much as possible — "bump" has no precise meaning to me.

I do see your point that ocular-publish version-beta feels unclear, though...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can't use two words as the second argument is currently used as a custom NPM dist tag. This can of course change because it's not actively used by any repo and it seems like we are going with publishConfig.tag instead (I think that's a better approach).

Otherwise, does version-only-beta sound better?

modules/dev-tools/docs/cli/ocular-publish.md Show resolved Hide resolved
@Pessimistress Pessimistress merged commit 964bc6c into master Apr 24, 2024
4 checks passed
@Pessimistress Pessimistress deleted the x/publish branch April 24, 2024 18:44
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.

3 participants