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

ci: Publish jsapi-types patch release with v0.33 tag #5625

Merged

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Jun 17, 2024

Copy link
Member

@devinrsmith devinrsmith left a comment

Choose a reason for hiding this comment

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

I would call this is a workaround for #5572 as opposed to a "fix"; ideally, we'd plumb appropriate metadata through the job to handle this more generally.

- Should no longer clobber the `latest` tag
- Fixes deephaven#5572
@mofojed
Copy link
Member Author

mofojed commented Jun 18, 2024

I'll make a more general workaround and put it on main using the branch name.

@@ -113,5 +113,5 @@ jobs:
if: ${{ startsWith(github.ref, 'refs/heads/release/v') }}
env:
NODE_AUTH_TOKEN: ${{ secrets.DEEPHAVENBOT_NPM_TOKEN }}
run: npm publish --tag latest web/client-api/types/build/deephaven-jsapi-types-*.tgz
run: npm publish --tag v0.33 web/client-api/types/build/deephaven-jsapi-types-*.tgz
Copy link
Member

Choose a reason for hiding this comment

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

To make sure I understand a bit more: a tag here is not the same as npm "version" - it's an additional qualifier that points to a version?

Can a single version have multiple tags?

Looking at https://www.npmjs.com/package/@deephaven/jsapi-types?activeTab=versions, I see the tags latest, nightly, and canary. Are we planning to introduce a new convention for all npm releases, with a tag that is v<major>.<minor>?

https://docs.npmjs.com/adding-dist-tags-to-packages seems to recommend against this:

We recommend avoiding dist-tags that start with a number or the letter "v".

Copy link
Member Author

@mofojed mofojed Jun 18, 2024

Choose a reason for hiding this comment

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

Correct; we do the v<major>.<minor> tagging on the web-client-ui packages: https://www.npmjs.com/package/@deephaven/code-studio?activeTab=versions
But it could be any arbitrary name. Yea npm recommends against it, but not sure what else to name them? We could give them the same name as the Enterprise releases they coincide with (e.g. vplus), and maybe that would be a better fit (though ironically still wouldn't be recommended, since it starts with v)?

@mofojed
Copy link
Member Author

mofojed commented Jun 18, 2024

@devinrsmith actually I'm not sure I can make a more general case with how the release process is right now - it only publishes when the ref matches release/v*: if: ${{ startsWith(github.ref, 'refs/heads/release/v') }}

I don't think there's a way (yet) to distinguish between a main/latest release vs. a patch on a previous release, as at this point it's already a release/v* branch only.

@devinrsmith
Copy link
Member

@devinrsmith actually I'm not sure I can make a more general case with how the release process is right now - it only publishes when the ref matches release/v*: if: ${{ startsWith(github.ref, 'refs/heads/release/v') }}

I don't think there's a way (yet) to distinguish between a main/latest release vs. a patch on a previous release, as at this point it's already a release/v* branch only.

The more general solution we should discuss relative to #5631

@devinrsmith devinrsmith merged commit b26745b into deephaven:rc/v0.33.x Jun 18, 2024
14 of 16 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants